-
-
Notifications
You must be signed in to change notification settings - Fork 308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't special-case <stddef.h> #4289
Conversation
The commit ac6ef83 ("fix: build of imap/auth_sasl.c") claims to workaround a bug in sasl.h, which supposedly is missing an `#include <stddef.h>` for size_t. However, I can see the include in my system: alx@debian:~$ apt-file search sasl/sasl.h libsasl2-dev: /usr/include/sasl/sasl.h alx@debian:~$ grep stddef /usr/include/sasl/sasl.h #include <stddef.h> /* For size_t */ And the include has been there since 2012, according to a git-blame(1) on the source repository: alx@debian:/tmp/cyrus-sasl$ find | grep /sasl.h ./include/sasl.h alx@debian:/tmp/cyrus-sasl$ git blame ./include/sasl.h | grep stddef 67a188693 (Ken Murchison 2012-12-20 18:14:50 -0500 124) #include <stddef.h> /* For size_t */ alx@debian:/tmp/cyrus-sasl$ git show 67a188693 commit 67a188693796a14e3a76ac603104807fbbfddfc4 Author: Ken Murchison <murch@andrew.cmu.edu> Date: Thu Dec 20 18:14:50 2012 -0500 sasl.h: #include <stddef.h> for size_t on NetBSD diff --git a/include/sasl.h b/include/sasl.h index fef4d510..8b8a63fb 100755 --- a/include/sasl.h +++ b/include/sasl.h @@ -121,6 +121,8 @@ #ifndef SASL_H #define SASL_H 1 +#include <stddef.h> /* For size_t */ + /* Keep in sync with win32/common.mak */ #define SASL_VERSION_MAJOR 2 #define SASL_VERSION_MINOR 1 This seems to have been a red herring. There was probably some include problem somewhere else, and that change accidentally fixed it. Or maybe back then in 2017, some distributions still had an already-very old version of <sasl/sasl.h> which still didn't have the include. However it is, the include has been there for more than a decade now, and we don't need to workaround any bugs. Fixes: ac6ef83 ("fix: build of imap/auth_sasl.c") Link: <#4284> Cc: Richard Russon <rich@flatcap.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
Semi-scripted change: $ find * -type f \ | grep '\.[ch]$' \ | xargs grep -l 'include.*stddef' \ | while read f; do clang-format -i "$f"; done; Plus manually removing unrelated changes. Fixes: ac6ef83 ("fix: build of imap/auth_sasl.c") Closes: <#4284> Cc: Richard Russon <rich@flatcap.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
The macOS test fails:
That's weird, because that file includes <stdlib.h>, which is what provides mkdtemp(3), according to POSIX. |
Oh well, these ******** aren't POSIX-compliant, it seems. The macOS manual page for mkdtemp(3) says they provide it in <unistd.h>. POSIX: It's weird, because FreeBSD provides mkdtemp(3) in the right header. I'll add a commit adding an include <unistd.h> to that file. |
macOS violates POSIX.1-2017 by providing mkdtemp(3) in an incorrect header: <unistd.h> instead of <stdlib.h>. I've reported a bug to Apple. For now, let's include that header. Link: <https://www.apple.com/feedback/macos.html> Link: <#4289 (comment)> Link: <https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkdtemp.html> Link: <https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/mkdtemp.3.html> Link: <https://man.freebsd.org/cgi/man.cgi?query=mkdtemp&apropos=0&sektion=0&manpath=FreeBSD+14.0-RELEASE+and+Ports&arch=default&format=html> Signed-off-by: Alejandro Colomar <alx@kernel.org>
7066b19
to
3f3669f
Compare
It seems FreeBSD moved the prototype from <unistd.h> to <stdlib.h> somewhere between FreeBSD 9.0 (2012) and FreeBSD 10.0 (2014). macOS still lives in the past. :D |
macOS build passes after including <unistd.h>: https://github.com/neomutt/neomutt/actions/runs/8970958530 |
@flatcap If you're happy with it, let me know and I'll drop the last patch (or you can do it; as you prefer). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely, thanks!
Fixes: ac6ef83 ("fix: build of imap/auth_sasl.c")
Closes: #4284
Builds locally. Let's see what CI has to say.
Revisions:
v2
v2 changes: