Feature Request: Implement glibc reallocarray() function

[Darshit Shah]

Hi,

Glibc 2.26 introduced the reallocarray() function which acts as a safe
realloc counterpart to calloc(). I'd like to use this in GNU Wget but
since it is not easily available on all systems, I guess we should have
an implementation in gnulib. If others here agree, I'd like to see this
function within in gnulib.

If someone will willing to help me out / give me pointers on what would
be required to implement this within gnulib, I will happily port the
code from glibc to gnulib and submit a completed patch.

[Bruno Haible]

Hi Darshit,

Post by Darshit Shah
Glibc 2.26 introduced the reallocarray() function which acts as a safe
realloc counterpart to calloc(). ... If others here agree, I'd like to see this
function within in gnulib.

Yes, a portable replacement of this function belongs in gnulib.
It falls both in the categories "Enhancements of ISO C or POSIX functions" and
"Portable general use facilities" of the gnulib scope [1].

Post by Darshit Shah
If someone will willing to help me out / give me pointers on what would
be required to implement this within gnulib, I will happily port the
code from glibc to gnulib and submit a completed patch.

Nice! Thanks. You can start out by looking at the implementation of module 'strsep'
or by reading the chapter "Writing modules" [2].

Bruno

[1] https://www.gnu.org/software/gnulib/manual/html_node/Various-Kinds-of-Modules.html
[2] https://www.gnu.org/software/gnulib/manual/html_node/Writing-modules.html

[Paul Eggert]

Post by Bruno Haible
You can start out by looking at the implementation of module 'strsep'

Another suggestion: Gnulib reallocarray should use Gnulib's xalloc_oversized
macro instead of the glibc check_mul_overflow_size_t function. See the
xalloc-oversized module.

[Darshit Shah]

Hi Bruno,

Thanks for the guidelines. I've written a very rudimentary version of
the module. This is a barely working version, but I do not know what
else I require to complete the module. I know that this is missing:

1. An autoconf check
2. A way for reallocarray() to be defined within <stdlib.h>
3. Unit tests

Of these, I think I can write the unit tests without much trouble, but
for the other two, I don't know how to write them

The glibc version is implemented behind the feature macro, _GNU_SOURCE.
I am not sure how that should be implemented into the module in gnulib.

Post by Bruno Haible
Hi Darshit,

Post by Darshit Shah
Glibc 2.26 introduced the reallocarray() function which acts as a safe
realloc counterpart to calloc(). ... If others here agree, I'd like to see this
function within in gnulib.

Yes, a portable replacement of this function belongs in gnulib.
It falls both in the categories "Enhancements of ISO C or POSIX functions" and
"Portable general use facilities" of the gnulib scope [1].

Post by Darshit Shah
If someone will willing to help me out / give me pointers on what would
be required to implement this within gnulib, I will happily port the
code from glibc to gnulib and submit a completed patch.

Nice! Thanks. You can start out by looking at the implementation of module 'strsep'
or by reading the chapter "Writing modules" [2].
Bruno
[1] https://www.gnu.org/software/gnulib/manual/html_node/Various-Kinds-of-Modules.html
[2] https://www.gnu.org/software/gnulib/manual/html_node/Writing-modules.html

[Bruno Haible]

Hi Darshit,

Post by Darshit Shah
Thanks for the guidelines. I've written a very rudimentary version of
the module. This is a barely working version

The implementation (.c file) looks good. However, it could be formatted
in GNU style (like the rest of gnulib, with very few exceptions, e.g. fts.c).

The module name should IMO be 'reallocarray'. We use the suffix '-gnu' only
when there is ambiguity between POSIX and GNU behaviour (e.g. for getopt).

Post by Darshit Shah
1. An autoconf check
2. A way for reallocarray() to be defined within <stdlib.h>
3. Unit tests

And 4. A documentation file doc/glibc-functions/reallocarray.texi.

Post by Darshit Shah
for the other two, I don't know how to write them

1. Look at m4/strsep.m4.
2. grep for strsep and STRSEP in lib/string.in.h, m4/string_h.m4, modules/string.
We use the same idioms everywhere.

Post by Darshit Shah
The glibc version is implemented behind the feature macro, _GNU_SOURCE.
I am not sure how that should be implemented into the module in gnulib.

The gnulib modules make their symbols visible unconditionally. That is, a
user that includes the gnulib module 'reallocarray' should be able to use
reallocarray() without defining _GNU_SOURCE by themselves. This is done through
an invocation of AC_USE_SYSTEM_EXTENSIONS from within m4/reallocarray.m4.

Bruno

[Darshit Shah]

Hi Bruno,

Thanks for your help.
I've made the changes you mentioned and attached a new version of the
patch. Do let me know if something is missing.

I am not sure of how the documentation should be written. Specifically,
which platforms this module is targeting.

Post by Bruno Haible
Hi Darshit,

Post by Darshit Shah
Thanks for the guidelines. I've written a very rudimentary version of
the module. This is a barely working version

The implementation (.c file) looks good. However, it could be formatted
in GNU style (like the rest of gnulib, with very few exceptions, e.g. fts.c).
The module name should IMO be 'reallocarray'. We use the suffix '-gnu' only
when there is ambiguity between POSIX and GNU behaviour (e.g. for getopt).

Post by Darshit Shah
1. An autoconf check
2. A way for reallocarray() to be defined within <stdlib.h>
3. Unit tests

And 4. A documentation file doc/glibc-functions/reallocarray.texi.

Post by Darshit Shah
for the other two, I don't know how to write them

1. Look at m4/strsep.m4.
2. grep for strsep and STRSEP in lib/string.in.h, m4/string_h.m4, modules/string.
We use the same idioms everywhere.

Post by Darshit Shah
The glibc version is implemented behind the feature macro, _GNU_SOURCE.
I am not sure how that should be implemented into the module in gnulib.

The gnulib modules make their symbols visible unconditionally. That is, a
user that includes the gnulib module 'reallocarray' should be able to use
reallocarray() without defining _GNU_SOURCE by themselves. This is done through
an invocation of AC_USE_SYSTEM_EXTENSIONS from within m4/reallocarray.m4.
Bruno

[Darshit Shah]

Hi,

My last patch was incomplete. It did not contain the unit tests. I have
attached the updated patch file.

However, it does not work and fails the test. It seems to me that I have
not managed to correctly coerce stdlib.h into defining a prototype for
reallocarray(). Where did I go wrong / what did I miss?

Post by Bruno Haible
Hi Darshit,

Post by Darshit Shah
Thanks for the guidelines. I've written a very rudimentary version of
the module. This is a barely working version

The implementation (.c file) looks good. However, it could be formatted
in GNU style (like the rest of gnulib, with very few exceptions, e.g. fts.c).
The module name should IMO be 'reallocarray'. We use the suffix '-gnu' only
when there is ambiguity between POSIX and GNU behaviour (e.g. for getopt).

Post by Darshit Shah
1. An autoconf check
2. A way for reallocarray() to be defined within <stdlib.h>
3. Unit tests

And 4. A documentation file doc/glibc-functions/reallocarray.texi.

Post by Darshit Shah
for the other two, I don't know how to write them

1. Look at m4/strsep.m4.
2. grep for strsep and STRSEP in lib/string.in.h, m4/string_h.m4, modules/string.
We use the same idioms everywhere.

Post by Darshit Shah
The glibc version is implemented behind the feature macro, _GNU_SOURCE.
I am not sure how that should be implemented into the module in gnulib.

The gnulib modules make their symbols visible unconditionally. That is, a
user that includes the gnulib module 'reallocarray' should be able to use
reallocarray() without defining _GNU_SOURCE by themselves. This is done through
an invocation of AC_USE_SYSTEM_EXTENSIONS from within m4/reallocarray.m4.
Bruno

[Paul Eggert]

It seems to me that I have not managed to correctly coerce stdlib.h into
defining a prototype for reallocarray(). Where did I go wrong / what did I miss?

Look at other GNU extensions in stdlib.in.h. canonicalize_file_name, say.

Also, please use GNU styile everywhere. No tabs. A space before paren in
function and macro calls.

Also, list the maintainer as "all", as this really isn't a single-person module.

[Bruno Haible]

Hi Darshit,

Looks quite complete now. Other than what Paul mentioned:

* You appear to be using tabs of widths 4. Therefore use "expand -t 4" to
untabify the files you modified.

* The documentation:
- Could you add an URL to the documentation of this function? (glibc
manual or, if not available, manpage from the LTP)
- The function is not present on any other system, I checked. Therefore the
text should read:

This function is missing on all non-glibc platforms:
glibc 2.25, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 11 2011-11, Cygwin, mingw, MSVC 14, Interix 3.5, BeOS.

* In the tests, you could add a check for the declaration of the function.
Look in test-strchrnul.c how we do it.

Post by Darshit Shah
However, it does not work and fails the test. It seems to me that I have
not managed to correctly coerce stdlib.h into defining a prototype for
reallocarray(). Where did I go wrong / what did I miss?

I don't see a problem. IMO
$ ./gnulib-tool --test reallocarray
should work fine. Let's debug it when you've resubmitted once more.

Bruno

[Paul Eggert]

Post by Bruno Haible
The function is not present on any other system, I checked.

It was introduced in OpenBSD 5.6 and was added to FreeBSD in 11.1.

Hmm, the NetBSD man page says it's available in NetBSD 8 in the _OPENBSD_SOURCE
namespace, which is news to me. This means the extensions module needs to define
yet another symbol (and the proposed module should depend on the extensions
module). I installed the attached to do the extensions-module part of this.

[Bruno Haible]

Post by Paul Eggert

Post by Bruno Haible
The function is not present on any other system, I checked.

It was introduced in OpenBSD 5.6 and was added to FreeBSD in 11.1.

Oops, my database of libc symbols per platform was not updated in a while.
Sorry.

Bruno

[Darshit Shah]

Hi Bruno,

Thanks for all your help. I've attached an updated version of the patch
which should now be (almost) complete. I've taken into account
everything that Paul and you mentioned in the last mail.

Post by Bruno Haible
Hi Darshit,
* You appear to be using tabs of widths 4. Therefore use "expand -t 4" to
untabify the files you modified.
- Could you add an URL to the documentation of this function? (glibc
manual or, if not available, manpage from the LTP)
- The function is not present on any other system, I checked. Therefore the
glibc 2.25, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 11 2011-11, Cygwin, mingw, MSVC 14, Interix 3.5, BeOS.
* In the tests, you could add a check for the declaration of the function.
Look in test-strchrnul.c how we do it.

Post by Darshit Shah
However, it does not work and fails the test. It seems to me that I have
not managed to correctly coerce stdlib.h into defining a prototype for
reallocarray(). Where did I go wrong / what did I miss?

I don't see a problem. IMO
$ ./gnulib-tool --test reallocarray
should work fine. Let's debug it when you've resubmitted once more.
Bruno

[Paul Eggert]

+This function is missing on all non-glibc platforms: glibc 2.25

No, as I mentioned it's present in the three main BSD platforms. Please change
"all non-glibc" to "many", and update the version numbers for FreeBSD etc. to
match my earlier email.

[Darshit Shah]

You're right. I missed that part. Here's an updated version

Post by Paul Eggert

+This function is missing on all non-glibc platforms: glibc 2.25

No, as I mentioned it's present in the three main BSD platforms.
Please change "all non-glibc" to "many", and update the version
numbers for FreeBSD etc. to match my earlier email.

[Paul Eggert]

Thanks, a few more things (hopefully we're getting to the end now).

"missing on many platforms" lists reallocarray as missing in FreeBSD 11.1,
NetBSD 8.0, and OpenBSD 5.6. But it's not missing in those versions: those are
the versions it was first introduced.

Please fit ChangeLog entries to 79 columns if possible.

When talking about a function FOO, say just "FOO", not "FOO()". Use the latter
form only when talking about calls to FOO with zero arguments.

The test could be easier to read and more useful if we got rid of the 'eight'
function and tried realloc_array with several different sizes. Also, there's no
need to free the array. E.g.,

size_t n;
for (n = 2; n != 0; n <<= 1)
if (reallocarray (NULL, (size_t) -1 / n + 1, n))
return 1;
return 0;

[Darshit Shah]

Post by Paul Eggert
Thanks, a few more things (hopefully we're getting to the end now).
"missing on many platforms" lists reallocarray as missing in FreeBSD
11.1, NetBSD 8.0, and OpenBSD 5.6. But it's not missing in those
versions: those are the versions it was first introduced.

I'm a little confused here. The manpage for NetBSD[1] states:
" The reallocarray() function first appeared in OpenBSD 5.6.
reallocarray() was redesigned in NetBSD 8 as reallocarr(3). For compati-
bility reasons it's available since NetBSD 8 in the _OPENBSD_SOURCE
namespace."

This page was written in 2015. However, on netbsd.org, NetBSD 7.1 is the
current latest release. Also, when exactly do we consider reallocarray
being available in NetBSD since the page only states that the redesigned
version, reallocarr (note the missing 'ay') is available from 8.0. I am
not very familiar with the different BSDs and am unable to make much
sense out of this.

Also, I've changed the line for Mac OSX to version 10.13 which is the
latest version since I can't find any evidence of the function being
ever available on that platform

Post by Paul Eggert
Please fit ChangeLog entries to 79 columns if possible.
When talking about a function FOO, say just "FOO", not "FOO()". Use
the latter form only when talking about calls to FOO with zero
arguments.

Done

Post by Paul Eggert
The test could be easier to read and more useful if we got rid of the
'eight' function and tried realloc_array with several different sizes.
Also, there's no need to free the array. E.g.,
size_t n;
for (n = 2; n != 0; n <<= 1)
if (reallocarray (NULL, (size_t) -1 / n + 1, n))
return 1;
return 0;

I had originally copied the test from test-calloc-gnu.c. I've updated it
with the sample you provided and added another test to ensure that the
correct errno is set as well.

[Paul Eggert]

This page was written in 2015. However, on netbsd.org, NetBSD 7.1 is the current
latest release. Also, when exactly do we consider reallocarray being available
in NetBSD since the page only states that the redesigned version, reallocarr
(note the missing 'ay') is available from 8.0. I am not very familiar with the
different BSDs and am unable to make much sense out of this.

NetBSD 8 has not been released yet. The man page was written well in advance of
the release. I wouldn't worry about the possibility that reallocarray is removed
before NetBSD 8 is official.

Also, it's now called "macOS", not "Mac OS X".

[Paul Eggert]

what is the first NetBSD version that includes `reallocarray`?

I would say NetBSD 8. But that's not what the text is about. The text doesn't
talk about the first NetBSD version that includes 'reallocarray'. It talks about
the last version that is missing 'reallocarray'. As of this writing this is 7.1.

Are there any other blockers for this patch?

Not that I recall.

[Darshit Shah]

Post by Paul Eggert

what is the first NetBSD version that includes `reallocarray`?

I would say NetBSD 8. But that's not what the text is about. The text
doesn't talk about the first NetBSD version that includes
'reallocarray'. It talks about the last version that is missing
'reallocarray'. As of this writing this is 7.1.

Are there any other blockers for this patch?

Not that I recall.

Here is the updated patch. I hope everything is now in order this can be
merged into master

[Paul Eggert]

I hope everything is now in order this can be merged into master

Thanks, it's close enough. I installed it along with the attached minor cleanups.

[Bruno Haible]

Hi Darshit,

Thank you very much for your patience, to go through these details for your
first major gnulib contributions! I appreciate it a lot.

Bruno

[Darshit Shah]

Post by Bruno Haible
Hi Darshit,
Thank you very much for your patience, to go through these details for your
first major gnulib contributions! I appreciate it a lot.
Bruno

I'd like to thank everyone here who helped me along the way making sure
that the patch was made perfectly. And also to deal with my broken MUA
configuration for a while, not to mention my editor which is set to use
tabs and I kept forgetting to change it for the gnulib code.

Now, I have a better idea of how ti work with gnulib even for my own
projects.

[Bruno Haible]

what is the first NetBSD version that includes `reallocarray`?

You can infer this from the CVS log of the NetBSD sources:
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/stdlib/reallocarr.c

Bruno

[Bruno Haible]

Post by Paul Eggert
When talking about a function FOO, say just "FOO", not "FOO()". Use the latter
form only when talking about calls to FOO with zero arguments.

Except that in the module description, we use 'FOO()' as a kind of markup,
in order to insert a hyperlink to the POSIX standard where possible.

In this case, as long as 'reallocarray' is not standardized by POSIX, this does
not matter, though.

Bruno