bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: test-getlogin false failure in non login shells


From: Guilherme de Almeida Suckevicz
Subject: Re: test-getlogin false failure in non login shells
Date: Thu, 15 May 2014 11:12:35 -0300

Hi Pádraig,

I completely OK with it, my doubt was with the assert calls!
Thank you for the help! ;D

Thanks,
Guilherme Almeida.


2014-05-14 18:16 GMT-03:00 Pádraig Brady <address@hidden>:
On 05/14/2014 03:03 PM, Guilherme de Almeida Suckevicz wrote:
> Hi Pádraig,
>
> Thanks for the help! So, Adjusting just the part on non windows platforms, I arrive here.
> I'm just with doubt in the ASSERT calls.
>
> Please, if you can, take a look:
>
> diff --git a/tests/test-getlogin.c b/tests/test-getlogin.c
> index 197aed8..0a46267 100644
> --- a/tests/test-getlogin.c
> +++ b/tests/test-getlogin.c
> @@ -27,6 +27,11 @@ SIGNATURE_CHECK (getlogin, char *, (void));
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <unistd.h>
> +#include <pwd.h>
> +
> +#include <sys/stat.h>
> +#include <sys/types.h>
>
>  #include "macros.h"
>
> @@ -62,11 +67,29 @@ main (void)
>  #if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
>    /* Unix platform */
>    {
> -    const char *name = getenv ("LOGNAME");
> -    if (name == NULL || name[0] == '\0')
> -      name = getenv ("USER");
> -    if (name != NULL && name[0] != '\0')
> -      ASSERT (strcmp (buf, name) == 0);
> +    const char *tty;
> +    struct stat stat_buf;
> +    struct passwd *pwd;
> +
> +    tty = ttyname (STDIN_FILENO);
> +    if (tty == NULL)
> +      {
> +       if (errno == ENOTTY)
> +         {
> +           fprintf (stderr, "Skipping test: stdin is not a tty.\n");
> +           return 77;
> +         }
> +       /* Some other error, call assert. */
> +       ASSERT (0);
> +      }
> +
> +    if (stat (tty, &stat_buf) < 0)
> +      ASSERT (0);
> +
> +    pwd = getpwuid (stat_buf.st_uid);
> +    ASSERT (! (pwd == NULL));
> +
> +    ASSERT (strcmp (pwd->pw_name, buf) == 0);
>    }
>  #endif
>  #if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
>
>
> Sorry for any mistake!
>
> Thank you very much!
> Guilherme Almeida.
>
>
> 2014-05-13 18:47 GMT-03:00 Pádraig Brady <address@hidden <mailto:address@hidden>>:
>
>     On 05/13/2014 08:39 PM, Guilherme de Almeida Suckevicz wrote:
>     > Hi Pádraig,
>     >
>     > Thanks for the answer! I will work on a patch for that.
>     >
>     > I think the better way to fix this is check the getlogin(3) against the
>     > owner of the controlling terminal, this is, using the ttyname(3), stat(2)
>     > and the field pw_name of getpwuid(3).
>     >
>     > Is it right!?
>
>     Well first of all the test is really only checking the windows replacement function,
>     so I wouldn't jump through many hoops to correlate the actual getlogin() independently.
>     Anyway let's see what combos of env variables we have...
>
>     tp1:~$ python -c "import os, pwd; print os.environ.get('USER'), os.environ.get('LOGNAME'), os.environ.get('SUDO_USER'), os.getlogin(), pwd.getpwuid(os.stat(os.ttyname(0)).st_uid).pw_name"
>     padraig padraig None padraig padraig
>     tp1:~$ sudo !!
>     root root padraig padraig padraig
>
>     tp1:~$ ssh address@hidden
>     address@hidden ~]# su - padraig
>     tp2:~$ python -c "import os, pwd; print os.environ.get('USER'), os.environ.get('LOGNAME'), os.environ.get('SUDO_USER'), os.getlogin(), pwd.getpwuid(os.stat(os.ttyname(0)).st_uid).pw_name"
>     padraig padraig None root root
>     tp2:~$ sudo !!
>     root root padraig root root
>
>     So we see the last 2 (your method and getlogin()) correlate for each case here.
>     The problematic cases are when LOGNAME/USER are different from getlogin(),
>     and I don't see any other combo of env variables to identify those cases,
>     so it seems your method is best for direct correlation.
>
>     Now on non windows systems we're not actually replacing any code,
>     so the simple solution would be on non windows to not care
>     about the returned value at all.
>
>     I.E. I'd just do the following to remove the problematic code
>     which is invalid in all the above caes. If we actually implement
>     getlogin() on non windows we can jump though hoops then to
>     do the correlation.
>
>     diff --git a/tests/test-getlogin.c b/tests/test-getlogin.c
>     index 197aed8..7b19f8c 100644
>     --- a/tests/test-getlogin.c
>     +++ b/tests/test-getlogin.c
>     @@ -59,16 +59,6 @@ main (void)
>          }
>
>        /* Compare against the value from the environment.  */
>     -#if !((defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__)
>     -  /* Unix platform */
>     -  {
>     -    const char *name = getenv ("LOGNAME");
>     -    if (name == NULL || name[0] == '\0')
>     -      name = getenv ("USER");
>     -    if (name != NULL && name[0] != '\0')
>     -      ASSERT (strcmp (buf, name) == 0);
>     -  }
>     -#endif
>      #if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
>        /* Native Windows platform.
>           Note: This test would fail on Cygwin in an ssh session, because sshd
>
>     thanks,
>     Pádraig.
>
>

So you went the route of adding rather than deleting code. Fair enough.
I tweaked a bit in the attached which I;ll apply soon if you're OK with it.
It passes these here:

  ./gnulib-tool --create-testdir --with-tests --test getlogin
  sudo !!

thanks,
Pádraig.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]