-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
nilfs-utils: force malloc(0) realloc(0) to be valid #58056
Conversation
I think there should be a comment in the code with this link, and a mention which environments need this fix (I do not ask for making it optional — just for a bit more clarity and context) Probably a bash comment |
6d7e8ee
to
4266bff
Compare
updated! |
Do I understand correctly that the workaround is not really about |
@7c6f434c as I understand it, yes, ideally the code itself should be patched to be portable across different libcs |
# https://www.uclibc.org/FAQ.html#gnu_malloc | ||
# https://www.gnu.org/software/autoconf/manual/autoconf.html#index-AC_005fFUNC_005fMALLOC-454 | ||
# the upstream source should be patched to avoid needing this | ||
"ac_cv_func_malloc_0_nonnull=yes" |
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.
Since this is a hack, we should make it conditional on cross compiling. Otherwise we might be overriding legitimate detection.
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.
I wonder if it was possible to provide the correct information about the host in a more generic way to autotools, since I have seen this problem with ac_cv_func_malloc_0_nonnull
in many places.
@illegalprime I think the comment is misleading then — it is not a problem of detection (malloc(0) is NULL), it is a problem of drawing wrong inferences (the actual features needed by the code are there).. @matthewbauer I think it is libc-dependent, not cross-compilation-dependent? And apparently for non-glibc libcs we have this functionality is never useful. |
@7c6f434c yeah that makes sense, should it be conditional on |
We can also add musl to the list:
Then most cross-compiling use-cases should be covered.
|
My point about inferences about other features was more about the comment than about exact condition — that it is not because we assume things about Also, doesn't this also apply to uClibc without cross-compilation? |
@7c6f434c Yes it is libc-dependent. The only difference is, that in the non-crosscompiling builds, autotools can interfere the right value at buildtime. For now we could set this value for libc's where we tested the value. |
… and autodetection for |
@7c6f434c Yeah. I don't think we would need to care about uclibc given that Musl support in nixpkgs and in general is better. |
Ah, I misestimated the relevance of uClibc link in the comment |
This is
some of those are my fault, but should they all be changed or maybe moved into |
@Mic92 so what about putting this in stdenv under a isCross & isGlibc (isMusl) flag? |
@illegalprime sounds fine to me to set this, if we have |
when I add those flags to stdenv some new things fail to compile
relevant commit: illegalprime@4e79d1d maybe autotools has a way to set default flags even if they might not exist (cc @Mic92) |
zlib is actually not using autotools at all, but a custom shell script that emulates autotools in some ways. To get around this, don't put the value in "configureFlags", but instead just set it in the environment. I believe autotools picks it up as well. |
I also remember environment variables to work. |
Thank you for your contributions.
|
I marked this as stale due to inactivity. → More info |
This has been merged in #106761 |
Motivation for this change
configure
is not sure whether the host system will have amalloc
andrealloc
that returns a valid pointer when given0
so it assumes no and fails, this forces configure to assume yes.https://www.uclibc.org/FAQ.html#gnu_malloc
https://www.gnu.org/software/autoconf/manual/autoconf.html#index-AC_005fFUNC_005fMALLOC-454
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)