[pacman-dev] [PATCH v3 2/3] util-common.h: Add ARRAYSIZE macro

[Pierre Neidhardt]

On 15-10-21 17:28:11, Allan McRae wrote:
> On 21/10/15 17:15, Pierre Neidhardt wrote:
> > On 15-10-21 14:49:21, Allan McRae wrote:
> >> On 20/10/15 23:30, Pierre Neidhardt wrote:
> >>> Signed-off-by: Pierre Neidhardt <ambrevar at gmail.com>
> >>> ---
> >>>  src/common/util-common.h | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/src/common/util-common.h b/src/common/util-common.h
> >>> index a2093be..373db19 100644
> >>> --- a/src/common/util-common.h
> >>> +++ b/src/common/util-common.h
> >>> @@ -34,6 +34,8 @@ char *safe_fgets(char *s, int size, FILE *stream);
> >>>  char *strndup(const char *s, size_t n);
> >>>  #endif
> >>>  
> >>> +#define ARRAYSIZE(a) (sizeof (a) / sizeof (a[0]))
> >>> +
> >>>  #endif /* _PM_UTIL_COMMON_H */
> >>>  
> >>
> >> This is needed nowhere in libalpm, so put this in src/pacman/util.c at
> >> the top of the file.  And you might as well use it in the three places
> >> that could use it in pacman while you are at it.
> >>
> >> Allan
> > 
> > How do you define the appropriate spots where it can be used? I believe alpm
> > could use the macro in many places as well. For instance, both pacman and alpm
> > have numerous PATH_MAX referencing the size of an array (beside its
> > initialization of course).
> 
> PATH_MAX is used all over the place for storing paths.  They usually get
> filled with a snprintf result and then have the length stored.  I don't
> see a use for this define there.

PATH_MAX is not a great example as it is fairly idiomatic. But you can use
ARRAYSIZE for any string manipulation, e.g.:

	snprintf(s, ARRAYSIZE(s), ...

This way you can choose to restrict the explicit 'sizeof' calls to types. As was
originally suggested in HACKING.

It's a simple matter of style consistency, and since this was not done before I
don't think we should change it.

> > I endorse the use of ARRAYSIZE, but I must say it is quite inconsistent with
> > what has been done until now. Suggestions:
> > 
> > * We replace every static array size reference with a call to the macro. Lots of
> > work and error prone.
> 
> I can see use doing the size of calculation in three places:
> 
> $ git grep sizeof | grep "\["
> src/pacman/pacman.c: for(i = 0; i < sizeof(signals) /
> sizeof(signals[0]); i++) {
> src/pacman/sync.c: for(j = 0; j < sizeof(glob_skips) /
> sizeof(glob_skips[0]); j++) {
> src/pacman/util.c: static const int unitcount = sizeof(labels) /
> sizeof(labels[0]);

Wrong regex:

	$ git grep "sizeof *([^)]*) */"
	lib/libalpm/pkghash.c:	loopsize = sizeof(prime_list) / sizeof(*prime_list);
	src/pacman/pacman.c:	for(i = 0; i < sizeof(signals) / sizeof(signals[0]); i++) {
	src/pacman/sync.c:				for(j = 0; j < sizeof(glob_skips) / sizeof(glob_skips[0]); j++) {
	src/pacman/util.c:	static const int unitcount = sizeof(labels) / sizeof(labels[0]);

So yes, alpm has *one* use for it! :D

-- 
Pierre Neidhardt

Americans are people who insist on living in the present, tense.