(oh, and you'll have to switch to plain text mode or it'll bounce!) On Fri, Jun 16, 2023 at 3:25 PM enh <enh@xxxxxxxxxx> wrote: > > use `git format-patch -1` to generate a patch file, and then you can follow the instructions: > > - Send patches in diff -u format, inline inside the email message. If > you're worried about your mailer breaking the patch, the send it > both inline and as an attachment. You may find it useful to employ > git-send-email(1) and git-format-patch(1). > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING > > (note that gmail is very much a mailer that will break the patch, so you will need to send as an attachment.) > > On Fri, Jun 16, 2023 at 2:10 PM Zijun Zhao <zijunzhao@xxxxxxxxxx> wrote: >> >> Hi Alejandro, >> I already created the patch attached in the last email.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 Mon, Jun 5, 2023 at 12:03 PM Zijun Zhao <zijunzhao@xxxxxxxxxx> wrote: >> > >> > 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
