Avoid alloca in libpsl and tests #231
Conversation
rockdaboot
force-pushed
the
avoid-alloca
branch
from
ed636bd
to
3465a3d
Compare
| # include <alloca.h> | ||
| #elif defined _WIN32 | ||
| # include <malloc.h> | ||
| # include <malloc.h> |
There was a problem hiding this comment.
I assume you mean stdlib.h here? This is a glibc-ism.
There was a problem hiding this comment.
Regarding https://learn.microsoft.com/de-de/cpp/c-runtime-library/reference/malloc?view=msvc-170, malloc() requires stdlib.h and malloc.h. The first is included further up, unconditionally.
Glibc only requires stdlib.h.
There was a problem hiding this comment.
Right but for non Windows the canonical definition is stdlib.h -- malloc.h may not exist and isn't documented as the correct way to do it.
It does exist on glibc but it doesn't exist on macOS, which caused a build failure.
There was a problem hiding this comment.
My bad, the include was in the #ifndef _WIN32 path, should have be in the #else path. Fixed it and force-pushed.
There was a problem hiding this comment.
@eli-schwartz Can you check again (see my comment #231 (comment)) and unblock the PR if OK?
There was a problem hiding this comment.
Yup, sorry, lost track of that.
rockdaboot
force-pushed
the
avoid-alloca
branch
2 times, most recently
from
21227e9
to
272cd35
Compare
Reported-by: @rsbeckerca (github.com)
Reported-by: @rsbeckerca (github.com)
eli-schwartz
force-pushed
the
avoid-alloca
branch
from
272cd35
to
49219cc
Compare
eli-schwartz
dismissed
their stale review
macOS build failures resolved, looks good now
|
Rebased in order to refresh CI checks, everything looks good. Apologies for the holdup. |
alloca()needs special treatment for Windows and possibly other systems. The implementation also is compiler-specific.alloca()in general is considered "dangerous" (similar to variable-length arrays (VLAs)).This PR drops the usage of
alloca()for the aforementioned reasons.