[pacman-dev] [PATCH v3 2/3] util-common.h: Add ARRAYSIZE macro
Allan McRae
allan at archlinux.org
Wed Oct 21 08:54:12 UTC 2015
On 21/10/15 17:55, Pierre Neidhardt wrote:
> 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.
So we are both saying it should not get used in string manipulation?
Which is good because I would not accept changes to that...
>>> 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
>
OK - keep it in util-common then. Will you submit a patch with those
changed to use ARRAYSIZE? Or should I just patchcount++ myself?
A
More information about the pacman-dev
mailing list