This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] y2038: linux: Provide __timerfd_gettime64 implementation


Hi Joseph,

> On Sat, 7 Dec 2019, Lukasz Majewski wrote:
> 
> > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c
> > b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644
> > index 0000000000..498605369b
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c
> > @@ -0,0 +1,69 @@
> > +/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +   Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.  
> 
> New files need to have a descriptive comment before the copyright
> notice, and no "Contributed by" line, and only have previous years in
> the copyright notice if genuinely incorporating copyrightable content
> from previous files from those years.

Thanks for the hint.

I will add the descriptive comment and treat this file as a "new" one -
with copyright description similar to newly added files.

> 
> > +int
> > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value)
> > +{
> > +  if (fd < 0)
> > +    return INLINE_SYSCALL_ERROR_RETURN_VALUE (EBADF);  
       ^^^^^^^^^ - [*]

> 
> Why?  In general, such checks are only needed in userspace if correct 
> function semantics means not passing such a case to the kernel at
> all, as opposed to letting the kernel return an error for it.

I took this approach from futimens.c (from
sysdeps/unix/sysv/linux). This is how the wrong fd is handled.
According to manual [1] the EBADF shall be returned.

The manual for timerfd_gettime [2] also states that wrong fd shall
cause the EBADF return value.

And yes - you are right - the Linux kernel checks [3] if fd is valid and
returns either -EBADF or -EINVAL.
Considering the above - I will remove the check [*] from
__timerfd_gettime64 and send v2.

> 
> The same comments apply to patch 2/2.
> 

Ok.


Links:

[1] - https://linux.die.net/man/3/futimens
[2] - https://linux.die.net/man/2/timerfd_gettime
[3] - https://elixir.bootlin.com/linux/v5.4/source/fs/timerfd.c#L374



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Attachment: pgpSe3chAnmqz.pgp
Description: OpenPGP digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]