[PATCH] Fix getlogin() to check only stdin to get a valid tty

[PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Hello,

In my hobby OS [1] which uses Newlib C as its libc, I noticed that the 
GNU command 'logname' does output nothing when it is redirected or pipe'd.

The current getlogin() implementation [2] forces the three primary file 
descriptors (stdin, stdout and stderr) to be a valid tty before checking 
the utmp file, otherwise it returns NULL. This makes impossible to 
redirect (to a file or to a pipe), the output of a program that is using 
this function because one of its file descriptors won't be a tty.

See:

# cat test.c
#include <stdio.h>

int main(void)
{
	char *user;

	if(user = getlogin()) {
		printf("%s\n", user);
	}

	return 0;
}

# ./test
root
# ./test > xx
# cat xx
# ./test | cat
#

To fix this, I thought that only checking if stdin is a valid tty would 
be enough.

Does this sound reasonable to you?
Thanks.



diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
index da4f47a95..e646bcb08 100644
--- a/newlib/libc/unix/getlogin.c
+++ b/newlib/libc/unix/getlogin.c
@@ -16,9 +16,7 @@ getlogin ()
    extern char *ttyname ();
    char *tty;

-  if (((tty = ttyname (0)) == 0)
-      || ((tty = ttyname (1)) == 0)
-      || ((tty = ttyname (2)) == 0))
+  if ((tty = ttyname (0)) == 0)
      return 0;

    if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)


1. https://www.fiwix.org
2. 
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/unix/getlogin.c;hb=HEAD

-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Torbjorn SVENSSON]

On 2023-07-12 19:33, Jordi Sanfeliu via Newlib wrote:
> Hello,
> 
> In my hobby OS [1] which uses Newlib C as its libc, I noticed that the 
> GNU command 'logname' does output nothing when it is redirected or pipe'd.
> 
> The current getlogin() implementation [2] forces the three primary file 
> descriptors (stdin, stdout and stderr) to be a valid tty before checking 
> the utmp file, otherwise it returns NULL. This makes impossible to 
> redirect (to a file or to a pipe), the output of a program that is using 
> this function because one of its file descriptors won't be a tty.

I think your analysis is wrong. See below for reasons why.

> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> index da4f47a95..e646bcb08 100644
> --- a/newlib/libc/unix/getlogin.c
> +++ b/newlib/libc/unix/getlogin.c
> @@ -16,9 +16,7 @@ getlogin ()
>     extern char *ttyname ();
>     char *tty;
> 
> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))

These 3 lines of code checks if one of stdin, stdout or stderr is 
connected to a terminal device. If the return value of ttyname is 0, it 
means that there is no terminal device connected to that fd.
As I read the code, it first tries with stdin. If stdin is closed or 
redirected, it tries with stdout instead and then lastly, falls back to 
trying with stderr. If none of the 3 fd's provides a terminal device, 
then the getlogin will return 0.

As a result, doing your change would force the process to have a 
connected stdin or the getlogin call would return 0.

> +  if ((tty = ttyname (0)) == 0)
>       return 0;
> 
>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
> 
> 
> 1. https://www.fiwix.org
> 2. 
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/unix/getlogin.c;hb=HEAD
> 


While looking at the code pointed to in link 2, I'm a bit puzzled about 
the following lines:

static char buf[10];
...
strncpy (buf, utmp_buf.ut_user, sizeof (utmp_buf.ut_user));

As buf is 10 bytes, I suppose sizeof(utmp_buf.ut_user) should never be 
allowed to exceed 10 bytes, but in the newlib source tree, I find these 
files that define a size for utmp_buf.ut_user:

./newlib/libc/sys/sysvi386/sys/utmp.h: 8
./winsup/cygwin/include/cygwin/utmp.h: 16

I'm not sure if this getlogin.c file is included in a cygwin build, but 
if it is, I think there is a buffer overflow here.


Kind regards,
Torbjörn

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Brian Inglis]

On 2023-07-12 12:50, Torbjorn SVENSSON wrote:
> 
> 
> On 2023-07-12 19:33, Jordi Sanfeliu via Newlib wrote:
>> Hello,
>>
>> In my hobby OS [1] which uses Newlib C as its libc, I noticed that the GNU 
>> command 'logname' does output nothing when it is redirected or pipe'd.
>>
>> The current getlogin() implementation [2] forces the three primary file 
>> descriptors (stdin, stdout and stderr) to be a valid tty before checking the 
>> utmp file, otherwise it returns NULL. This makes impossible to redirect (to a 
>> file or to a pipe), the output of a program that is using this function 
>> because one of its file descriptors won't be a tty.
> 
> I think your analysis is wrong. See below for reasons why.
> 
>> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
>> index da4f47a95..e646bcb08 100644
>> --- a/newlib/libc/unix/getlogin.c
>> +++ b/newlib/libc/unix/getlogin.c
>> @@ -16,9 +16,7 @@ getlogin ()
>>     extern char *ttyname ();
>>     char *tty;
>>
>> -  if (((tty = ttyname (0)) == 0)
>> -      || ((tty = ttyname (1)) == 0)
>> -      || ((tty = ttyname (2)) == 0))
> 
> These 3 lines of code checks if one of stdin, stdout or stderr is connected to a 
> terminal device. If the return value of ttyname is 0, it means that there is no 
> terminal device connected to that fd.
> As I read the code, it first tries with stdin. If stdin is closed or redirected, 
> it tries with stdout instead and then lastly, falls back to trying with stderr. 
> If none of the 3 fd's provides a terminal device, then the getlogin will return 0.
> 
> As a result, doing your change would force the process to have a connected stdin 
> or the getlogin call would return 0.
> 
>> +  if ((tty = ttyname (0)) == 0)
>>       return 0;
>>
>>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
>>
>>
>> 1. https://www.fiwix.org
>> 2. 
>> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/unix/getlogin.c;hb=HEAD
>>
> 
> 
> While looking at the code pointed to in link 2, I'm a bit puzzled about the 
> following lines:
> 
> static char buf[10];
> ...
> strncpy (buf, utmp_buf.ut_user, sizeof (utmp_buf.ut_user));
> 
> As buf is 10 bytes, I suppose sizeof(utmp_buf.ut_user) should never be allowed 
> to exceed 10 bytes, but in the newlib source tree, I find these files that 
> define a size for utmp_buf.ut_user:
> 
> ./newlib/libc/sys/sysvi386/sys/utmp.h: 8
> ./winsup/cygwin/include/cygwin/utmp.h: 16
> 
> I'm not sure if this getlogin.c file is included in a cygwin build, but if it 
> is, I think there is a buffer overflow here.

No worries: Cygwin getlogin/_r is defined as "C" in winsup/cygwin/uinfo.cc to 
deal with Windows and limits itself to UNLEN:
/usr/include/cygwin/limits.h:#define __LOGIN_NAME_MAX 256       /* equal to 
UNLEN defined in w32api/lmcons.h */
/usr/include/w32api/lmcons.h:#define UNLEN 256

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Torbjorn SVENSSON]

On 2023-07-12 22:06, Brian Inglis wrote:
> No worries: Cygwin getlogin/_r is defined as "C" in 
> winsup/cygwin/uinfo.cc to deal with Windows and limits itself to UNLEN:
> /usr/include/cygwin/limits.h:#define __LOGIN_NAME_MAX 256       /* equal 
> to UNLEN defined in w32api/lmcons.h */
> /usr/include/w32api/lmcons.h:#define UNLEN 256
> 

Thanks Brian for the clarification!

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Hello Torbjörn,

Thanks for you reply.

It seems to me that this code returns 0 as soon as one of the 3 fds is 
0, regardless if the rest are valid tty names.

I think that what you meant is this:

         if ((tty = ttyname (0)) == 0)
             if ((tty = ttyname (1)) == 0)
                 if ((tty = ttyname (2)) == 0)
                     return 0;

which IMO would be even a better patch.

What do you think?

Best regards.


On 7/12/23 20:50, Torbjorn SVENSSON wrote:

>> -  if (((tty = ttyname (0)) == 0)
>> -      || ((tty = ttyname (1)) == 0)
>> -      || ((tty = ttyname (2)) == 0))
> 
> These 3 lines of code checks if one of stdin, stdout or stderr is 
> connected to a terminal device. If the return value of ttyname is 0, it 
> means that there is no terminal device connected to that fd.
> As I read the code, it first tries with stdin. If stdin is closed or 
> redirected, it tries with stdout instead and then lastly, falls back to 
> trying with stderr. If none of the 3 fd's provides a terminal device, 
> then the getlogin will return 0.

-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Torbjorn SVENSSON]

On 2023-07-13 10:06, Jordi Sanfeliu via Newlib wrote:
> Hello Torbjörn,
> 
> Thanks for you reply.
> 
> It seems to me that this code returns 0 as soon as one of the 3 fds is 
> 0, regardless if the rest are valid tty names.
> 
> I think that what you meant is this:
> 
>          if ((tty = ttyname (0)) == 0)
>              if ((tty = ttyname (1)) == 0)
>                  if ((tty = ttyname (2)) == 0)
>                      return 0;
> 
> which IMO would be even a better patch.
> 
> What do you think?

Actually, you're right. Sorry for the confusion.
I took a sneak peak at how glibc does this and there is this comment:

   /* Get name of tty connected to fd 0.  Return NULL if not a tty or
      if fd 0 isn't open.  Note that a lot of documentation says that
      getlogin() is based on the controlling terminal---what they
      really mean is "the terminal connected to standard input".  The
      getlogin() implementation of DEC Unix, SunOS, Solaris, HP-UX all
      return NULL if fd 0 has been closed, so this is the compatible
      thing to do.  Note that ttyname(open("/dev/tty")) on those
      systems returns /dev/tty, so that is not a possible solution for
      getlogin().  */

Based on this comment, I guess it would be sane to drop the check on 
stdout and stderr, but it would have the consequence that you are not 
able to pipe some data on stdin to the application that calls getlogin 
as it would fail in that scenario.

I'm not a maintainer of newlib so I don't really have anything to say 
about what path you decide to go.

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Hello,

So we have two possible patches to solve this situation:

My first original patch that only checks if stdin is a tty and seems to 
match with glibc comments provided by Torbjörn. The only problem is that 
it won't work in the scenario when someone pipes some data on stdin to 
the application:

$ echo | ./test
$


diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
index da4f47a95..e646bcb08 100644
--- a/newlib/libc/unix/getlogin.c
+++ b/newlib/libc/unix/getlogin.c
@@ -16,9 +16,7 @@ getlogin ()
    extern char *ttyname ();
    char *tty;

-  if (((tty = ttyname (0)) == 0)
-      || ((tty = ttyname (1)) == 0)
-      || ((tty = ttyname (2)) == 0))
+  if ((tty = ttyname (0)) == 0)
      return 0;

    if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)



And a patch based on the comments of Torbjörn that checks all three fds 
before returning NULL:

diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
index da4f47a95..5a3f172e7 100644
--- a/newlib/libc/unix/getlogin.c
+++ b/newlib/libc/unix/getlogin.c
@@ -16,10 +16,10 @@ getlogin ()
    extern char *ttyname ();
    char *tty;

-  if (((tty = ttyname (0)) == 0)
-      || ((tty = ttyname (1)) == 0)
-      || ((tty = ttyname (2)) == 0))
-    return 0;
+  if ((tty = ttyname (0)) == 0)
+      if ((tty = ttyname (1)) == 0)
+          if ((tty = ttyname (2)) == 0)
+              return 0;

    if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
      return 0;


Any thoughts?
Thanks.


On 7/13/23 18:25, Torbjorn SVENSSON wrote:

> I took a sneak peak at how glibc does this and there is this comment:
> 
>    /* Get name of tty connected to fd 0.  Return NULL if not a tty or
>       if fd 0 isn't open.  Note that a lot of documentation says that
>       getlogin() is based on the controlling terminal---what they
>       really mean is "the terminal connected to standard input".  The
>       getlogin() implementation of DEC Unix, SunOS, Solaris, HP-UX all
>       return NULL if fd 0 has been closed, so this is the compatible
>       thing to do.  Note that ttyname(open("/dev/tty")) on those
>       systems returns /dev/tty, so that is not a possible solution for
>       getlogin().  */
> 
> Based on this comment, I guess it would be sane to drop the check on 
> stdout and stderr, but it would have the consequence that you are not 
> able to pipe some data on stdin to the application that calls getlogin 
> as it would fail in that scenario.
> 
> I'm not a maintainer of newlib so I don't really have anything to say 
> about what path you decide to go.

-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jeff Johnston]

[-- Attachment #1: Type: text/plain, Size: 3165 bytes --]

I think that when in doubt, going with glibc is the right answer.

I'll give Corinna time to pipe in if she wishes, otherwise, I'll check in
the glibc-like patch.

-- Jeff J.

On Thu, Jul 13, 2023 at 12:58 PM Jordi Sanfeliu via Newlib <
newlib@sourceware.org> wrote:

> Hello,
>
> So we have two possible patches to solve this situation:
>
> My first original patch that only checks if stdin is a tty and seems to
> match with glibc comments provided by Torbjörn. The only problem is that
> it won't work in the scenario when someone pipes some data on stdin to
> the application:
>
> $ echo | ./test
> $
>
>
> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> index da4f47a95..e646bcb08 100644
> --- a/newlib/libc/unix/getlogin.c
> +++ b/newlib/libc/unix/getlogin.c
> @@ -16,9 +16,7 @@ getlogin ()
>     extern char *ttyname ();
>     char *tty;
>
> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))
> +  if ((tty = ttyname (0)) == 0)
>       return 0;
>
>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
>
>
>
> And a patch based on the comments of Torbjörn that checks all three fds
> before returning NULL:
>
> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> index da4f47a95..5a3f172e7 100644
> --- a/newlib/libc/unix/getlogin.c
> +++ b/newlib/libc/unix/getlogin.c
> @@ -16,10 +16,10 @@ getlogin ()
>     extern char *ttyname ();
>     char *tty;
>
> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))
> -    return 0;
> +  if ((tty = ttyname (0)) == 0)
> +      if ((tty = ttyname (1)) == 0)
> +          if ((tty = ttyname (2)) == 0)
> +              return 0;
>
>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
>       return 0;
>
>
> Any thoughts?
> Thanks.
>
>
> On 7/13/23 18:25, Torbjorn SVENSSON wrote:
>
> > I took a sneak peak at how glibc does this and there is this comment:
> >
> >    /* Get name of tty connected to fd 0.  Return NULL if not a tty or
> >       if fd 0 isn't open.  Note that a lot of documentation says that
> >       getlogin() is based on the controlling terminal---what they
> >       really mean is "the terminal connected to standard input".  The
> >       getlogin() implementation of DEC Unix, SunOS, Solaris, HP-UX all
> >       return NULL if fd 0 has been closed, so this is the compatible
> >       thing to do.  Note that ttyname(open("/dev/tty")) on those
> >       systems returns /dev/tty, so that is not a possible solution for
> >       getlogin().  */
> >
> > Based on this comment, I guess it would be sane to drop the check on
> > stdout and stderr, but it would have the consequence that you are not
> > able to pipe some data on stdin to the application that calls getlogin
> > as it would fail in that scenario.
> >
> > I'm not a maintainer of newlib so I don't really have anything to say
> > about what path you decide to go.
>
> --
> Jordi Sanfeliu
> FIBRANET Network Services Provider
> https://www.fibranet.cat
>
>

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Corinna Vinschen]

On Jul 13 15:01, Jeff Johnston wrote:
> I think that when in doubt, going with glibc is the right answer.
> 
> I'll give Corinna time to pipe in if she wishes, otherwise, I'll check in
> the glibc-like patch.

Sounds right to me.


Corinna

> 
> -- Jeff J.
> 
> On Thu, Jul 13, 2023 at 12:58 PM Jordi Sanfeliu via Newlib <
> newlib@sourceware.org> wrote:
> 
> > Hello,
> >
> > So we have two possible patches to solve this situation:
> >
> > My first original patch that only checks if stdin is a tty and seems to
> > match with glibc comments provided by Torbjörn. The only problem is that
> > it won't work in the scenario when someone pipes some data on stdin to
> > the application:
> >
> > $ echo | ./test
> > $
> >
> >
> > diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> > index da4f47a95..e646bcb08 100644
> > --- a/newlib/libc/unix/getlogin.c
> > +++ b/newlib/libc/unix/getlogin.c
> > @@ -16,9 +16,7 @@ getlogin ()
> >     extern char *ttyname ();
> >     char *tty;
> >
> > -  if (((tty = ttyname (0)) == 0)
> > -      || ((tty = ttyname (1)) == 0)
> > -      || ((tty = ttyname (2)) == 0))
> > +  if ((tty = ttyname (0)) == 0)
> >       return 0;
> >
> >     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
> >
> >
> >
> > And a patch based on the comments of Torbjörn that checks all three fds
> > before returning NULL:
> >
> > diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> > index da4f47a95..5a3f172e7 100644
> > --- a/newlib/libc/unix/getlogin.c
> > +++ b/newlib/libc/unix/getlogin.c
> > @@ -16,10 +16,10 @@ getlogin ()
> >     extern char *ttyname ();
> >     char *tty;
> >
> > -  if (((tty = ttyname (0)) == 0)
> > -      || ((tty = ttyname (1)) == 0)
> > -      || ((tty = ttyname (2)) == 0))
> > -    return 0;
> > +  if ((tty = ttyname (0)) == 0)
> > +      if ((tty = ttyname (1)) == 0)
> > +          if ((tty = ttyname (2)) == 0)
> > +              return 0;
> >
> >     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
> >       return 0;
> >
> >
> > Any thoughts?
> > Thanks.
> >
> >
> > On 7/13/23 18:25, Torbjorn SVENSSON wrote:
> >
> > > I took a sneak peak at how glibc does this and there is this comment:
> > >
> > >    /* Get name of tty connected to fd 0.  Return NULL if not a tty or
> > >       if fd 0 isn't open.  Note that a lot of documentation says that
> > >       getlogin() is based on the controlling terminal---what they
> > >       really mean is "the terminal connected to standard input".  The
> > >       getlogin() implementation of DEC Unix, SunOS, Solaris, HP-UX all
> > >       return NULL if fd 0 has been closed, so this is the compatible
> > >       thing to do.  Note that ttyname(open("/dev/tty")) on those
> > >       systems returns /dev/tty, so that is not a possible solution for
> > >       getlogin().  */
> > >
> > > Based on this comment, I guess it would be sane to drop the check on
> > > stdout and stderr, but it would have the consequence that you are not
> > > able to pipe some data on stdin to the application that calls getlogin
> > > as it would fail in that scenario.
> > >
> > > I'm not a maintainer of newlib so I don't really have anything to say
> > > about what path you decide to go.
> >
> > --
> > Jordi Sanfeliu
> > FIBRANET Network Services Provider
> > https://www.fibranet.cat
> >
> >

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Stefan Tauner]

On Thu, 13 Jul 2023 18:57:32 +0200
Jordi Sanfeliu via Newlib <newlib@sourceware.org> wrote:

> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))
> -    return 0;
> +  if ((tty = ttyname (0)) == 0)
> +      if ((tty = ttyname (1)) == 0)
> +          if ((tty = ttyname (2)) == 0)
> +              return 0;

IMHO the bug arose simply because of a || vs. && confusion. You can
just replace the || with && and have exactly the same effect as this
patch but it would be more customary.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jeff Johnston]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

Original patch pushed, thanks.

-- Jeff J.

On Wed, Jul 12, 2023 at 1:34 PM Jordi Sanfeliu via Newlib <
newlib@sourceware.org> wrote:

> Hello,
>
> In my hobby OS [1] which uses Newlib C as its libc, I noticed that the
> GNU command 'logname' does output nothing when it is redirected or pipe'd.
>
> The current getlogin() implementation [2] forces the three primary file
> descriptors (stdin, stdout and stderr) to be a valid tty before checking
> the utmp file, otherwise it returns NULL. This makes impossible to
> redirect (to a file or to a pipe), the output of a program that is using
> this function because one of its file descriptors won't be a tty.
>
> See:
>
> # cat test.c
> #include <stdio.h>
>
> int main(void)
> {
>         char *user;
>
>         if(user = getlogin()) {
>                 printf("%s\n", user);
>         }
>
>         return 0;
> }
>
> # ./test
> root
> # ./test > xx
> # cat xx
> # ./test | cat
> #
>
> To fix this, I thought that only checking if stdin is a valid tty would
> be enough.
>
> Does this sound reasonable to you?
> Thanks.
>
>
>
> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> index da4f47a95..e646bcb08 100644
> --- a/newlib/libc/unix/getlogin.c
> +++ b/newlib/libc/unix/getlogin.c
> @@ -16,9 +16,7 @@ getlogin ()
>     extern char *ttyname ();
>     char *tty;
>
> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))
> +  if ((tty = ttyname (0)) == 0)
>       return 0;
>
>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
>
>
> 1. https://www.fiwix.org
> 2.
>
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/unix/getlogin.c;hb=HEAD
>
> --
> Jordi Sanfeliu
> FIBRANET Network Services Provider
> https://www.fibranet.cat
>
>

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Thank you very much.


On 7/17/23 21:06, Jeff Johnston wrote:
> Original patch pushed, thanks.
> 
> -- Jeff J.
> 


-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Hello Jeff,

I'm reviewing the patch you applied to getlogin.c and it seems to me 
that there is an extra opening parenthesis.

You forgot the diff line:

  +  if ((tty = ttyname (0)) == 0)

Can you confirm it?

Regards.



On 7/17/23 21:06, Jeff Johnston wrote:
> Original patch pushed, thanks.
> 
> -- Jeff J.
> 

-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jeff Johnston]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

Yes, sorry.  Your patch didn't apply so I quickly manually changed it and
messed it up.
Fix applied.

-- Jeff J.

On Tue, Jul 18, 2023 at 3:08 AM Jordi Sanfeliu <jordi@fibranet.cat> wrote:

> Hello Jeff,
>
> I'm reviewing the patch you applied to getlogin.c and it seems to me
> that there is an extra opening parenthesis.
>
> You forgot the diff line:
>
>   +  if ((tty = ttyname (0)) == 0)
>
> Can you confirm it?
>
> Regards.
>
>
>
> On 7/17/23 21:06, Jeff Johnston wrote:
> > Original patch pushed, thanks.
> >
> > -- Jeff J.
> >
>
> --
> Jordi Sanfeliu
> FIBRANET Network Services Provider
> https://www.fibranet.cat
>
>

Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

[Jordi Sanfeliu]

Perfect!


On 7/18/23 19:44, Jeff Johnston wrote:
> Yes, sorry.  Your patch didn't apply so I quickly manually changed it 
> and messed it up.
> Fix applied.
> 
> -- Jeff J.

-- 
Jordi Sanfeliu
FIBRANET Network Services Provider
https://www.fibranet.cat