|
From: | Guilherme de Almeida Suckevicz |
Subject: | Re: test-getlogin false failure in non login shells |
Date: | Thu, 15 May 2014 11:12:35 -0300 |
On 05/14/2014 03:03 PM, Guilherme de Almeida Suckevicz wrote:
> Hi Pádraig,
>
> 2014-05-13 18:47 GMT-03:00 Pádraig Brady <address@hidden <mailto:address@hidden>>:> 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.
>
>
So you went the route of adding rather than deleting code. Fair enough.>
> 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.
>
>
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.
[Prev in Thread] | Current Thread | [Next in Thread] |