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 <p...@draigbrady.com>:
> 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 <p...@draigbrady.com <mailto:
> p...@draigbrady.com>>:
> >
> > 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 root@tp2
> > [root@tp2 ~]# 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.
>
>
