Hi Alejandro, I already created the patch attached below. How to add it to this page? Do I need to use git push, or just provide the patch? Could you help put it in .SH VERSIONS? Thank you! Best, Zijun Zhao On Fri, May 19, 2023 at 3:42 PM Alejandro Colomar <alx.manpages@xxxxxxxxx> wrote: > > On 5/20/23 00:21, enh wrote: > > On Fri, May 19, 2023 at 1:03 AM Alejandro Colomar > > <alx.manpages@xxxxxxxxx> wrote: > [...] > > >> You'll also need to add _Nullable to the function prototypes in the > >> SYNOPSIS. > > > > heh, funnily enough, zijunzhao's adding exactly those annotations to > > the bionic headers, which is how we stumbled across this. > > :) > > > > >> BTW, I see that glibc still requires nonnull in gettimeofday(3). It's > >> only settimeofday(3) that is nullable. > > > > yeah, but the _kernel_ allows null in both (see kernel/time/time.c), > > and this is section 2 of the man page. it's unclear to me whether > > that's worth calling out up here, or should be down in the > > Linux-specific section? (this page seems to be written as if it cares > > about Hurd? "On a non-Linux kernel, with glibc ...".) > > Yes, we care about hurd, and non-g libc, in this page and others. > Normally, I would be cautious, and specify the strictest prototype of > all implementations about which I care, but in this case, glibc seems > unnecessarily restrictive, and I'm ready to add _Nullable if the kernel > and some libc support that. However, as a caution, please add a > (CAVEATS?) section documenting that glibc doesn't allow NULL there. > Or maybe keep the SYNOPSIS without _Nullable, and have some VERSIONS > section specifying that other libcs and the kernel support NULL. > > > > >> See: > >> > >> > >> $ grepc gettimeofday /usr/include/*/sys/time.h > >> /usr/include/x86_64-linux-gnu/sys/time.h:67: > >> extern int gettimeofday (struct timeval *__restrict __tv, > >> void *__restrict __tz) __THROW __nonnull ((1)); > >> > >> > >> /usr/include/x86_64-linux-gnu/sys/time.h:75: > >> # define gettimeofday __gettimeofday64 > >> > >> > >> $ grepc settimeofday /usr/include/*/sys/time.h > >> /usr/include/x86_64-linux-gnu/sys/time.h:86: > >> extern int settimeofday (const struct timeval *__tv, > >> const struct timezone *__tz) > >> __THROW; > >> > >> > >> /usr/include/x86_64-linux-gnu/sys/time.h:106: > >> # define settimeofday __settimeofday64 > >> > >> > >> And while NULL may be non-UB, the manual page is not very clear on why > >> someone would want to call these functions with NULL. Could you > >> please also explain why would someone want to call these functions > >> with NULL? (Let's discuss it in the list, and then we see what > >> wording we use for the page.) > > > > so there are definitely several orders of magnitude more users of a > > null timezone --- most callers don't want that (not least because > > glibc seems to not fill it out anyway? musl certainly doesn't, > > implementing gettimeofday() in terms of clock_gettime() instead). > > > > the users for a null time (and non-null timezone; i haven't seen > > anyone -- not even a test -- pass null for both!) are interesting. > > Android's init uses settimeofday() with only a timezone, but -- > > although the timezone to use is settable in device-specific init > > scripts -- for devices in our tree, that's actually always 0. there's > > other proprietary (non-Google) code that uses gettimeofday() with only > > a timezone, and i'm honestly not sure whether it's deliberate that > > it's asking the _kernel_ for the timezone or not (i'm guessing they > > want the device timezone rather than the app timezone, but i do not > > believe there are devices where the _kernel_ timezone matches the > > device timezone?). certainly both of these use cases seem worth > > following up on, and i will do. > > Please. And thanks for the details! > > > > > but looking at musl (which ignores tz in both cases) it seems like the > > most impactful change would be to fix "The functions gettimeofday() > > and settimeofday() can get and set the time as well as a timezone" :-) > > :-) > > Makes sense; although it would be worth being clear about if that's > really true, and if libcs do support that as well as the kernel. > > > > > it also seems like an improvement to really call out the libc/kernel > > differences more clearly though. > > Sure. Many pages have a dedicated subsection. Feel free to add it > to this page. I'd put it in .SH VERSIONS. > > $ grep -rn 'C library/kernel differences' man* > man2/sigprocmask.2:120:.SS C library/kernel differences > man2/setreuid.2:168:.SS C library/kernel differences > man2/brk.2:126:.SS C library/kernel differences > man2/sigwaitinfo.2:114:.SS C library/kernel differences > man2/gethostname.2:122:.SS C library/kernel differences > man2/getcpu.2:73:.SS C library/kernel differences > man2/fork.2:263:.SS C library/kernel differences > man2/epoll_wait.2:258:.SS C library/kernel differences > man2/wait4.2:157:.SS C library/kernel differences > man2/stat.2:355:.SS C library/kernel differences > man2/sigsuspend.2:74:.SS C library/kernel differences > man2/access.2:40: /* But see C library/kernel differences, below */ > man2/access.2:285:.SS C library/kernel differences > man2/clock_getres.2:354:.SS C library/kernel differences > man2/getpid.2:57:.SS C library/kernel differences > man2/setfsgid.2:57:.SS C library/kernel differences > man2/getgroups.2:142:.SS C library/kernel differences > man2/sigaction.2:946:.SS C library/kernel differences > man2/setresuid.2:100:.SS C library/kernel differences > man2/readv.2:290:.SS C library/kernel differences > man2/readv.2:342:.SS Historical C library/kernel differences > man2/open.2:1393:.SS C library/kernel differences > man2/setgid.2:58:.SS C library/kernel differences > man2/setfsuid.2:97:.SS C library/kernel differences > man2/ptrace.2:2814:.SS C library/kernel differences > man2/poll.2:382:.SS C library/kernel differences > man2/sigreturn.2:130:.SS C library/kernel differences > man2/eventfd.2:267:.SS C library/kernel differences > man2/mmap.2:729:.SS C library/kernel differences > man2/_exit.2:104:.SS C library/kernel differences > man2/time.2:86:.SS C library/kernel differences > man2/pread.2:101:.SS C library/kernel differences > man2/seccomp.2:975:.IR "C library/kernel differences" . > man2/setuid.2:104:.SS C library/kernel differences > man2/sched_setaffinity.2:95:return 0 (but see "C library/kernel differences" below, > man2/sched_setaffinity.2:220:.SS C library/kernel differences > man2/select.2:559:.SS C library/kernel differences > man2/sigpending.2:52:.SS C library/kernel differences > man2/posix_fadvise.2:135:.SS C library/kernel differences > man2/gettimeofday.2:173:.SS C library/kernel differences > man2/uname.2:83:.SS C library/kernel differences > man2/clone.2:1576:.SS C library/kernel differences > man2/getpriority.2:185:.SS C library/kernel differences > man2/signalfd.2:331:.SS C library/kernel differences > man2/timer_create.2:225:.SS C library/kernel differences > man2/wait.2:464:.SS C library/kernel differences > man2/nice.2:80:.SS C library/kernel differences > man2/chmod.2:303:.SS C library/kernel differences > man2/seteuid.2:114:.SS C library/kernel differences > man3/getcwd.3:217:.SS C library/kernel differences > man3/killpg.3:98:.SS C library/kernel differences > man3/mq_notify.3:174:.SS C library/kernel differences > man3/mq_open.3:269:.SS C library/kernel differences > man3/sigqueue.3:118:.SS C library/kernel differences > man3type/epoll_event.3type:37:.SS C library/kernel differences > man7/man-pages.7:402:.I "C library/kernel differences" > > Cheers, > Alex > > > > >> Thanks! > >> Alex > >> > >> -- > >> <http://www.alejandro-colomar.es/> > >> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 > >> > > -- > <http://www.alejandro-colomar.es/> > GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
From 4df5bd711ec42de300cfe1c78589c0e1b48e73cb Mon Sep 17 00:00:00 2001 From: Zijun Zhao <zijunzhao@google.com> Date: Mon, 5 Jun 2023 11:45:49 -0700 Subject: [PATCH] Modify the documentation of gettimeofday() We find tv arg is allowed to be null in bionic so make the documentation more clear. --- man2/gettimeofday.2 | 1 + 1 file changed, 1 insertion(+) diff --git a/man2/gettimeofday.2 b/man2/gettimeofday.2 index 9d134fa49..43f3fd9ff 100644 --- a/man2/gettimeofday.2 +++ b/man2/gettimeofday.2 @@ -175,6 +175,7 @@ On some architectures, an implementation of .BR gettimeofday () is provided in the .BR vdso (7). +The kernel accepts null for both time and timezone. The timezone argument is ignored by glibc and musl, and not passed to/from the kernel. Android's bionic passes the timezone argument to/from the kernel, but Android does not update the kernel timezone based on the device timezone in Settings, so the kernel's timezone is typically UTC. .SH STANDARDS .TP .BR gettimeofday () -- 2.41.0.rc0.172.g3f132b7071-goog
