Re: [PATCH] maint.mk: add syntax-check to avoid char[PATH_MAX]

[Jim Meyering]

Eric Blake wrote:
> On 06/23/2011 03:02 AM, Jim Meyering wrote:
>> Eric Blake wrote:
>>> POSIX allows PATH_MAX to be undefined.  And even if you use the
>>> gnulib pathmax module, where "pathmax.h" guarantees a definition,
>>> the definition might not be constant or might be so large as to
>>> be wasteful or cause stack overflows.  PATH_MAX should only be
>>> used as a limit or hueristic, not an array size.
>
>>>
>>> +# Even if you use pathmax.h to guarantee that PATH_MAX is defined, it might
>>> +# not be constant.  In general, use PATH_MAX as a limit, not an array
>>> +# allocation bound.
>>> +sc_prohibit_path_max_array:
>>> +   @prohibit='\[PATH''_MAX'                                        \
>>> +   halt='Avoid arrays of size PATH_MAX'                            \
>>> +     $(_sc_search_regexp)
>>
>> I like it.
>> As you and Paul discussed, some projects may well
>> want to disable the test, but that is easy enough.
>>
>> However, I would suggest a more permissive regexp, e.g.,
>>
>
>>
>> We could detect those, too:
>>
>>         @prohibit='(\balloca *\([^)]*|\[[^]]*)PATH''_MAX'            \
>>
>> but that would require a different diagnostic, since there
>> is no "array" involved, so it may be better to leave that for
>> a separate stack-size-related test.
>
> How about this?  Yes, I'm proposing renaming the rule to better reflect
> its use.
>
> diff --git i/top/maint.mk w/top/maint.mk
> index 4408a4e..8c42093 100644
> --- i/top/maint.mk
> +++ w/top/maint.mk
> @@ -1135,11 +1135,11 @@ sc_Wundef_boolean:
>         $(_sc_search_regexp)
>
>  # Even if you use pathmax.h to guarantee that PATH_MAX is defined, it might
> -# not be constant.  In general, use PATH_MAX as a limit, not an array
> -# allocation bound.
> -sc_prohibit_path_max_array:
> -     @prohibit='\[PATH''_MAX'                                        \
> -     halt='Avoid arrays of size PATH_MAX'                            \
> +# not be constant, or might overflow a stack.  In general, use PATH_MAX as
> +# a limit, not an array or alloca size.
> +sc_prohibit_path_max_allocation:
> +     @prohibit='(\balloca *\([^)]*|\[[^]]*)PATH_MAX'                 \
> +     halt='Avoid allocations of size PATH_MAX'                       \

That looks fine.  Thanks.

Though I'd make one small change:
s/allocations/stack allocation/

While it's best to avoid PATH_MAX altogether (and hence to avoid *any*
allocations of related size), saying "stack allocation" better
describes the code that this check complains about.