Re: [screen-devel] Correctly format lock message if GECOS data is CSV

[Tore Sinding Bekkedal]

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 <address@hidden> wrote:
> Hey,
>
> thanks for patch, few notes inline
>
> On Mon, 28 Aug 2017 12:20:46 +0200
> Tore Sinding Bekkedal <address@hidden> 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