On Mon, Nov 3, 2025 at 11:48 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Oct 31, 2025 at 1:31 PM Jacob Champion
> <jacob.champion@enterprisedb.com> wrote:
> >
> > On Thu, Oct 23, 2025 at 2:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > I've drafted the patches for this item.
> >
> > Thanks!
> >
> > > The 0001 patch allows the packager to select the random source:
> > > "openssl" or "system", by using --with-random-source option. If it's
> > > omitted and OpenSSL is used (--with-openssl or --with-ssl=openssl),
> > > 'openssl' source is automatically chosen. The selected random source
> > > can be shown in read-only GUC parameter random_source.
>
> Thank you for the comments.
>
> >
> > I think 0001 is missing logic to switch pg_strong_random.c according
> > to the new #defines; selecting `system` still uses OpenSSL on my
> > machine.
>
> Fixed.
>
> >
> > > + --with-random-source=NAME
> > > + set random number source (system,openssl)
> >
> > Bikeshedding: should we make it clear in the name that this covers
> > only the "strong" random implementation, for cryptography? Maybe
> > --with-strong-random, or --with-crypt-random, or...
>
> Agreed, I renamed it to --with-strong-random and applied it to other
> related names too .
>
> >
> > > + AC_DEFINE([USE_RANDOM_SOURCE_OPENSSL] , 1, [Define to 1 to use OpenSSL libary for random number generation])
> >
> > autoreconf 2.69 is refusing to process these AC_DEFINEs on my machine,
> > and I think it's because of the space between the end bracket ']' and
> > the comma. Removing it fixes generation for me.
>
> Fixed.
>
> >
> > > @@ -3500,5 +3507,4 @@
> > > options => 'io_method_options',
> > > assign_hook => 'assign_io_method',
> > > },
> > > -
> > > ]
> >
> > (FWIW, I prefer the extra newline there; it's easier for me to read
> > and it matches the spacing at the top.)
>
> Fixed.
>
> >
> > > The 0002 patch supports getrandom() as a 'system' random source where
> > > available while keeping the method of reading /dev/urandom as a
> > > fallback option.
> >
> > > +AC_CHECK_HEADER([sys/random.h],
> > > + [AC_CHECK_FUNCS([getrandom],
> > > + [AC_DEFINE(HAVE_GETRANDOM, 1, [Define to 1 if you have getrandom])])])
> >
> > AC_CHECK_FUNCS([getrandom]) by itself should let autoconf take care of
> > the description; I think the extra AC_DEFINE there will duplicate some
> > handling.
> >
> > > +if cc.has_header('sys/random.h') and cc.has_function('getrandom',
> > > + args: test_c_args, prefix: '''
> > > +#include <sys/random.h>''')
> > > + cdata.set('HAVE_GETRANDOM', 1)
> > > +endif
> >
> > Some of the more complicated prefixes use multiline literals, but for
> > this case I think `prefix: '#include <sys/random.h'` would be more
> > readable. Also, is there a way to merge this check into the
> > check_funcs logic later on?
>
> Agreed with the above comments. I think we don't need a header check
> for sys/random.h actually. Checking the getrandom() function present
> seems to be sufficient in this case.
>
> I've attached the updated patches.
>
Rebased the patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
