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

[Allan McRae]

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.

> 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]);

I think you can replace those without too many errors creeping in.

> * Or we only use it in future code.
> * Or we don't use it at all and I remove it from my patch.