Make use of reallocarray(3) in libc.

[pfg]

Summary

libc already includes reallocarray(3) so its use is auto-consistent.

Try to use reallocarray(3) when we are using realloc(3) and there is a
multiplication involved. Attempt to use the unsigned nature of the
parameters unless they are a copy of something that is signed.

While here attempt to improve the initialization on some cases-

[ngie]

LGTM, sans stylistic changes and style fudging. The only blocking part (which was an issue before) is the nitems comment.

lib/libc/gen/glob.c

864 ↗

(On Diff #26139)

This comment (before and after) seems unnecessary. The manpages should be documenting this.

865 ↗

(On Diff #26139)

The void* cast seems unnecessary, before and after (the type is char**).

lib/libc/gen/scandir.c

85 ↗

(On Diff #26139)

• Please use a different name. This conflicts with the nitems macro in sys/param.h.
• Why move the initialization down? It doesn't cause any harm leaving it there, and it might inform gcc that the variable is indeed initialized at the right time.

119 ↗

(On Diff #26139)

The struct dirent** cast is unnecessary, as is the char* cast.

lib/libc/gen/setmode.c

178 ↗

(On Diff #26139)

Why the type change (I kind of understand why, but I want to be sure my assumptions match the intent).

lib/libc/gen/wordexp.c

238 ↗

(On Diff #26139)

Delete the extra space between ...OFFS ? and we..., while here?

This would also be a lot more readable if the assignment was done outside the if-statement and if (nmv == NULL) { was done separately.

lib/libc/iconv/citrus_esdb.c

266 ↗

(On Diff #26139)

This is ok, but I'd prefer this to be done as part of a separate cleanup commit.

lib/libc/regex/regcomp.c

1403 ↗

(On Diff #26139)

Delete the unnecessary casts while here?

lib/libc/rpc/getnetconfig.c

633 ↗

(On Diff #26139)

Same comment about the declaration and comparison being split up for readability that I made above.

lib/libc/stdio/ungetc.c

83 ↗

(On Diff #26139)

*groan* Someone was being overly clever (just for my own personal edification, I verified this)...

$ python2
Python 2.7.13 (default, Feb 21 2017, 01:53:52)
[GCC 4.2.1 Compatible FreeBSD Clang 3.9.1 (tags/RELEASE_391/final 289601)] on freebsd12
Type "help", "copyright", "credits" or "license" for more information.
>>> 0 << 1
0
>>> 1 << 1
2
>>> 2 << 1
4
>>> 3 << 1
6
>>> 4 << 1
8
>>> 5 << 1
10
>>> -1 << 1
-2

lib/libc/stdlib/getenv.c

276 ↗

(On Diff #26139)

Delete space between sizeof and ( while here?

310 ↗

(On Diff #26139)

Delete space between sizeof and ( while here?

[pfg]

Some notes here b/c Phabricator doesn't let me reply:

I am not sure it is good to remove all the casts from reallocarray: CERT C coding standard seem to have changed its mind.

MEM02-A. Do not cast the return value from malloc().
has been superceeded by:
MEM02-C. Immediately cast the result of a memory allocation function call
into a pointer to the allocated type.

---

About the spaces after sizeof (), I was respecting the style in the rest of the file, but you are right, it's better to use KNF while we are here.

• renaming nitems is a good idea.
• lazy assignment is recommended in our KNF.

[pfg]

lib/libc/gen/glob.c

864 ↗

(On Diff #26139)

The comment is pre-existing but I think it is a reminder that 'pglob->gl_pathv' can be NULL.

lib/libc/gen/scandir.c

85 ↗

(On Diff #26139)

It is better to initialize nitems (or numitems as it will be renamed, thanks) near to where it's used. If '(names ==NULL)' we will jump to 'fail' and avoid the initialization altogether.

lib/libc/gen/setmode.c

178 ↗

(On Diff #26139)

setlen is local (basically only used in a macro): unsigning it will give it a bit more space to grow (JIC) and will fit better the unsigned size_t from the reallocarray.

lib/libc/gen/wordexp.c

238 ↗

(On Diff #26139)

Yes about the space but changing to an if should be done in a different commit.

lib/libc/stdio/ungetc.c

83 ↗

(On Diff #26139)

FWIW, this change was inspired by OpenBSD :).

lib/libc/stdlib/getenv.c

276 ↗

(On Diff #26139)

Will do.

[ngie]

lib/libc/gen/setmode.c

178 ↗

(On Diff #26139)

Ok! Could this be done in a separate supporting commit please?

[pfg]

Update: just for reference ... I will be committing some
cleanups independently as suggested by ngie.

[pfg]

Forced update to restart the build.

[ngie]

In D9955#205916, @pfg wrote:

Forced update to restart the build.

The issue noted by HarborMaster is breakage with riscv, caused by rS315051. I doubt it's caused by your change.