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

[Amadeusz Sławiński]

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