Hi Aaron,
On Thu, Jun 18, 2026 at 01:17:40PM -0400, Aaron Merey wrote:
> On Thu, Jun 11, 2026 at 12:52 PM Mark Wielaard <[email protected]> wrote:
> > diff --git a/configure.ac b/configure.ac
> > index fbe039d5b43a..5779050b944b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -518,6 +518,13 @@ AC_CHECK_DECLS([reallocarray],[],[],
> > [#define _GNU_SOURCE
> > #include <stdlib.h>])
> >
> > +dnl for xmkstemp we need a random source
> > +AC_CHECK_DECLS([getentropy],[],[]
>
> Missing comma after the 2nd `[]`. I believe this still works as intended
> as-is since getentropy is declared in unistd.h which is included by default
> via AC_INCLUDES_DEFAULT when AC_CHECK_DECLS 4th arg is missing.
Nice catch. Added the comma.
> > +static inline int
> > +xrandom64 (uint64_t *r)
> > +{
> > + /* Prefer getentropy if it is available, fallback to getrandom, if
> > + both are missing, or if they fail try reading from /dev/urandom. */
> > +#if defined(HAVE_DECL_GETENTROPY)
>
> This will evaluate to true unconditionally since AC_CHECK_DECLS always
> sets this macro to either 1 or 0.
>
> > + if (getentropy (r, sizeof (*r)) == 0)
> > + return 0;
> > +#elif defined(HAVE_DECL_GETRANDOM)
>
> Currently this is dead code, but defined(HAVE_DECL_GETRANDOM) will similarly
> always evaluate to true if it were reached.
Doh. Fixed by doing #if HAVE_DECL_GETENTROPY #elif HAVE_DECL_GETRANDOM
instead. I did test this by removing the configure checks, didn't
occur to me that hid the incorrect check
> > + if (fdnew < 0 && errno == EEXIST)
> > {
> > - fnew = xstrdup (foutput);
> > - fdnew = open (fnew, O_WRONLY | O_CREAT, st.st_mode & ALLPERMS);
> > + /* OK, it already existed (or was a symlink). Try again, but now
> > + with the resolved path. */
> > + free (fnew);
> > + free (dirc);
> > + free (basec);
> > + close (dirfd);
> > + fnew = realpath (foutput == NULL ? fname : foutput, NULL);
> > + if (fnew == NULL)
> > + {
> > + error (0, errno, "Couldn't get realpath for %s",
> > + foutput == NULL ? fname : foutput);
> > + goto cleanup;
>
> This goto will cause double free/close of dirc, basec and dirfd. They are
> freed/closed right before the goto but cleanup assumes they're set.
Too many error paths :{
Added = NULL after the frees.
> Apart from these small fixes the patch LGTM.
Pushed with the above fixes.
Thanks,
Mark
