getlogin_r: return early when linux sentinel value is set

  When there is no login uid Linux sets /proc/self/loginid to the sentinel
value of 4294967295. If this is set we can return early and avoid
needlessly looking up the sentinel value in any configured nss
databases.
  

This is my first time submitting a patch, so any feedback would be
very much appreciated.

On Fri, Mar 16, 2018 at 11:18 AM, Jesse Hathaway <jesse@mbuki-mvuki.org> wrote:
> When there is no login uid Linux sets /proc/self/loginid to the sentinel
> value of 4294967295. If this is set we can return early and avoid
> needlessly looking up the sentinel value in any configured nss
> databases.
>
> diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c
> b/sysdeps/unix/sysv/linux/getlogin_r.c
> index 73ea14c8f9..43f55a2188 100644
> --- a/sysdeps/unix/sysv/linux/getlogin_r.c
> +++ b/sysdeps/unix/sysv/linux/getlogin_r.c
> @@ -55,6 +55,12 @@ __getlogin_r_loginuid (char *name, size_t namesize)
>     endp == uidbuf || *endp != '\0'))
>      return -1;
>
> +  /* If there is no login uid, linux sets /proc/self/loginid to the sentinel
> +     value of 4294967295, so check if the value is set and return early to
> +     avoid making unneeded nss lookups. */
> +  if (uid == 4294967295)
> +    return ENXIO;
> +
>    struct passwd pwd;
>    struct passwd *tpwd;
>    int result = 0;
  

On 17/03/2018 00:18, Jesse Hathaway wrote:
> When there is no login uid Linux sets /proc/self/loginid to the sentinel
> value of 4294967295. If this is set we can return early and avoid
> needlessly looking up the sentinel value in any configured nss
> databases.

The change is short enough so I think it won't require a copyright
assignment.  However it does require a ChangeLog entry, could you please
resend the patch with a proper one?

> 
> diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c
> b/sysdeps/unix/sysv/linux/getlogin_r.c
> index 73ea14c8f9..43f55a2188 100644
> --- a/sysdeps/unix/sysv/linux/getlogin_r.c
> +++ b/sysdeps/unix/sysv/linux/getlogin_r.c
> @@ -55,6 +55,12 @@ __getlogin_r_loginuid (char *name, size_t namesize)
>     endp == uidbuf || *endp != '\0'))
>      return -1;
> 
> +  /* If there is no login uid, linux sets /proc/self/loginid to the sentinel
> +     value of 4294967295, so check if the value is set and return early to
> +     avoid making unneeded nss lookups. */
> +  if (uid == 4294967295)
> +    return ENXIO;

I prefer to just use either (int)-1 or just 0xffffffff.  Also,
__getlogin_r_loginuid should set errno itself as for ERANGE instead
of just return its value (errno won't be set in this case and I think
it got it wrong for ENOMEM in this case).

> +
>    struct passwd pwd;
>    struct passwd *tpwd;
>    int result = 0;
>
  

On Mär 20 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 17/03/2018 00:18, Jesse Hathaway wrote:
>> When there is no login uid Linux sets /proc/self/loginid to the sentinel
>> value of 4294967295. If this is set we can return early and avoid
>> needlessly looking up the sentinel value in any configured nss
>> databases.
>
> The change is short enough so I think it won't require a copyright
> assignment.  However it does require a ChangeLog entry, could you please
> resend the patch with a proper one?
>
>> 
>> diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c
>> b/sysdeps/unix/sysv/linux/getlogin_r.c
>> index 73ea14c8f9..43f55a2188 100644
>> --- a/sysdeps/unix/sysv/linux/getlogin_r.c
>> +++ b/sysdeps/unix/sysv/linux/getlogin_r.c
>> @@ -55,6 +55,12 @@ __getlogin_r_loginuid (char *name, size_t namesize)
>>     endp == uidbuf || *endp != '\0'))
>>      return -1;
>> 
>> +  /* If there is no login uid, linux sets /proc/self/loginid to the sentinel
>> +     value of 4294967295, so check if the value is set and return early to
>> +     avoid making unneeded nss lookups. */
>> +  if (uid == 4294967295)
>> +    return ENXIO;
>
> I prefer to just use either (int)-1 or just 0xffffffff.

That should be (uid_t) -1.

Andreas.