Thank you very much for your comments, Amadeusz! You're right, strndup
is much better, I didn't know about it.
I think I have incorporated your suggestions here. Because it's
cosmetic I didn't see the need to panic if the allocation failed; I
just fall back to the previous behavior.
I'm not sure how the list feels about the brevity of collapsing
assignments and conditionals. I thought it was nice and concise at no
real cost to clarity.
Submitted for your approval:
diff --git a/src/socket.c b/src/socket.c
index 804592e..9dafeed 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -1093,21 +1093,31 @@ struct pwdata {
static void AskPassword(Message *m)
{
struct pwdata *pwdata;
char prompt[MAXSTR];
+ char * gecos_comma;
+ char * realname = NULL;
pwdata = calloc(1, sizeof(struct pwdata));
if (!pwdata)
Panic(0, "%s", strnomem);
pwdata->len = 0;
pwdata->m = *m;
D_processinputdata = (char *)pwdata;
D_processinput = PasswordProcessInput;
- snprintf(prompt, sizeof(prompt), "\ascreen used by %s%s<%s> on
%s.\r\nPassword: ", ppp->pw_gecos,
- ppp->pw_gecos[0] ? " " : "", ppp->pw_name, HostName);
+ /* if GECOS data is CSV, we only want the text before the first comma */
+ if ((gecos_comma = strchr(ppp->pw_gecos, ',')))
+ if (!(realname = strndup(ppp->pw_gecos, gecos_comma -
ppp->pw_gecos)))
+ gecos_comma = NULL; /* well, it was worth a shot. */
+
+ snprintf(prompt, sizeof(prompt), "\ascreen used by %s%s<%s> on
%s.\r\nPassword: ",
+ gecos_comma ? realname : ppp->pw_gecos,
+ ppp->pw_gecos[0] ? " " : "", ppp->pw_name, HostName);
+
+ free(realname);
AddStr(prompt);
}
#if ENABLE_PAM
On Wed, Aug 30, 2017 at 5:25 PM, Amadeusz Sławiński <am...@asmblr.net> wrote:
> Hey,
>
> thanks for patch, few notes inline
>
> On Mon, 28 Aug 2017 12:20:46 +0200
> Tore Sinding Bekkedal <tore...@gmail.com> wrote:
>
>> Hello - long time user, first time contributor.
>>
>> The screen lock message does not present GECOS data correctly if it -
>> as is tradition - is a comma-separated string; so rather than:
>>
>> screen used by Tore Sinding Bekkedal <toresbe> on pascal.
>>
>> I get:
>>
>> screen used by Tore Sinding Bekkedal,,, <toresbe> on pascal.
>>
>> Here's a patch which should fix that. I hope it's been formatted
>> correctly, sent to the right place, and meets the appropriate
>> standards - if not, please let me know.
>>
>> diff --git a/src/socket.c b/src/socket.c
>> index 804592e..4402016 100644
>> --- a/src/socket.c
>> +++ b/src/socket.c
>> @@ -1105,8 +1105,19 @@ static void AskPassword(Message *m)
>> D_processinputdata = (char *)pwdata;
>> D_processinput = PasswordProcessInput;
>>
>> - snprintf(prompt, sizeof(prompt), "\ascreen used by %s%s<%s> on
>> %s.\r\nPassword: ", ppp->pw_gecos,
>> + char * gecos_comma = strchr(ppp->pw_gecos, ',');
>> + char * realname = 0;
>
> Please declare variables at the beginning of function.
>
>> +
>> + if (gecos_comma) {
>> + realname = malloc(gecos_comma - ppp->pw_gecos);
>
> What happens if malloc fails? (look for "strnomem" in screen sources).
>
>> + memcpy(realname, ppp->pw_gecos, gecos_comma - ppp->pw_gecos);
>
> also instead of malloc/memcpy, I think it should be fine to just use
> strndup() as it is more concise, but as with malloc it can fail to
> allocate memory, so it needs to be handled.
>> + }
>> +
>> + snprintf(prompt, sizeof(prompt), "\ascreen used by %s%s<%s> on
>> %s.\r\nPassword: ",
>> + gecos_comma ? realname : ppp->pw_gecos,
>> ppp->pw_gecos[0] ? " " : "", ppp->pw_name, HostName);
>> +
>> + free(realname);
>> AddStr(prompt);
>> }
>>
>> regards,
>
>
> Best regards,
> Amadeusz Sławiński
>
--
Tore Sinding Bekkedal | http://gunkies.org/ | Mob: +47 91 85 95 08
"I never let my schooling interfere with my education." - Mark Twain
