[breaking change] gettimeofday 2nd argument incorrect in some targets #1354
Conversation
|
@gnzlbg: no appropriate reviewer found, use r? to override |
|
@semarie @strangelittlemonkey @leo60228 ping on the breaking changes for newlib, openbsd, and dragonflybsd |
gnzlbg
changed the title
|
cc @asomers since this is a breaking change on FreeBSD. |
|
no problem for me with a breaking change. but it seems to me that if you removed the bad definition of |
|
I wouldn't personally consider this worth a breaking change, but I think it's fine to land. I do think though that it requires being pretty vigilant after the release and ready to revert if it causes undue breakage. |
|
I'll do a release with this change only once its ready, and submit a PR to rust-lang/rust to update libc (we can do a crater run but... that might not find much), or see if these is any breakage after one nightly cycle. |
| @@ -1249,6 +1249,8 @@ f! { | |||
| } | |||
|
|
|||
| extern { | |||
| pub fn gettimeofday(tp: *mut ::timeval, | |||
There was a problem hiding this comment.
Why not combine FreeBSD's and Dragonfly's definitions in src/unix/bsd/freebsdlike/mod.rs ?
|
Looks good. FWIW, that's the Linux-specific time header in newlib, the actual one is here: https://github.com/devkitPro/newlib/blob/devkitA64/newlib/libc/include/sys/time.h#L370 |
|
Wait, I misunderstood what the commit did. It should be void* on newlib. |
The second argument of `gettimeofday` was a `*mut c_void` on all targets, but that type is incorrect in the following targets, where it should be a `*mut timezone` instead: On these other targets it appears that the signature of gettimeofday was incorrect (it takes a time-zone pointer instead of a void pointer): linux+gnu: http://man7.org/linux/man-pages/man2/gettimeofday.2.html freebsd: https://www.freebsd.org/cgi/man.cgi?query=gettimeofday&apropos=0&sektion=2&manpath=FreeBSD+11.2-stable&arch=default&format=html openbsd: https://man.openbsd.org/gettimeofday.2 android: https://github.com/ricardoquesada/android-ndk/blob/master/usr/include/sys/time.h dragonfly: https://www.dragonflybsd.org/cgi/web-man?command=gettimeofday§ion=2 This commit corrects the type on these targets, which is a breaking change. Due to how this API is commonly used (e.g. passing `ptr::null_mut` to the second argument), breakage should be minimal. Users wanting to support both versions can just write `ptr as *mut _` instead. Closes rust-lang#1338.
|
📌 Commit 759c837 has been approved by |
[breaking change] gettimeofday 2nd argument incorrect in some targets The second argument of `gettimeofday` was a `*mut c_void` on all targets, but that type is incorrect in the following targets, where it should be a `*mut timezone` instead: On these other targets it appears that the signature of gettimeofday was incorrect (it takes a time-zone pointer instead of a void pointer): *linux+gnu: http://man7.org/linux/man-pages/man2/gettimeofday.2.html *freebsd: https://www.freebsd.org/cgi/man.cgi?query=gettimeofday&apropos=0&sektion=2&manpath=FreeBSD+11.2-stable&arch=default&format=html *openbsd: https://man.openbsd.org/gettimeofday.2 *android: https://github.com/ricardoquesada/android-ndk/blob/master/usr/include/sys/time.h *dragonfly: https://www.dragonflybsd.org/cgi/web-man?command=gettimeofday§ion=2 This commit corrects the type on these targets, which is a breaking change. Due to how this API is commonly used (e.g. passing `ptr::null_mut` to the second argument), breakage should be minimal or non-existent (AFAICT only `time`, `libstd`, and `parking_lot` use this API, and they all should compile after this change). Users wanting to support both versions can just write `ptr as *mut _` instead. Closes #1338. --- On these targets, the signature of `gettimeofday` was correct (the second argument is a `void*`): * macosx: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/gettimeofday.2.html * linux+musl: http://git.musl-libc.org/cgit/musl/tree/include/sys/time.h#n11 * linux+newlib: https://chromium.googlesource.com/native_client/nacl-newlib/+/99fc6c167467b41466ec90e8260e9c49cbe3d13c/newlib/libc/include/sys/time.h#74 * netbsd: http://netbsd.gw.com/cgi-bin/man-cgi?gettimeofday+2.i386+NetBSD-8.0 * newlib: https://github.com/devkitPro/newlib/blob/devkitA64/newlib/libc/include/sys/time.h#L370 * solaris/illumos: https://illumos.org/man/3C/gettimeofday * emscripten: https://chromium.googlesource.com/external/github.com/kripken/emscripten/+/1.35.20/system/include/libc/sys/time.h#11 cc @alexcrichton
|
☀️ Test successful - checks-cirrus, checks-travis, status-appveyor |
The second argument of
gettimeofdaywas a*mut c_voidon all targets,but that type is incorrect in the following targets, where it should be
a
*mut timezoneinstead:On these other targets it appears that the signature of gettimeofday was incorrect (it takes a time-zone pointer instead of a void pointer):
*linux+gnu: http://man7.org/linux/man-pages/man2/gettimeofday.2.html
*freebsd: https://www.freebsd.org/cgi/man.cgi?query=gettimeofday&apropos=0&sektion=2&manpath=FreeBSD+11.2-stable&arch=default&format=html
*openbsd: https://man.openbsd.org/gettimeofday.2
*android: https://github.com/ricardoquesada/android-ndk/blob/master/usr/include/sys/time.h
*dragonfly: https://www.dragonflybsd.org/cgi/web-man?command=gettimeofday§ion=2
This commit corrects the type on these targets, which is a breaking change. Due
to how this API is commonly used (e.g. passing
ptr::null_mutto the secondargument), breakage should be minimal or non-existent (AFAICT only
time,libstd, andparking_lotuse this API, and they all should compile after this change). Users wanting to support both versions can just writeptr as *mut _instead.Closes #1338.
On these targets, the signature of
gettimeofdaywas correct (the second argument is avoid*):cc @alexcrichton