[PATCH v3] remove attribute access from regexec

[Martin Sebor]

On 8/26/21 10:24 AM, Paul Eggert wrote:
> On 8/26/21 8:06 AM, Martin Sebor wrote:
>>>
>>>> +#ifndef __ARG_NELTS
>>>> +# ifdef __ARG_NELTS
>>>
>>> A clear typo. The suggestion was to define and use _ARG_NELTS_ instead.
>>
>> __ARG_NELTS is  the name of macro in cdefs.h that you suggested so
>> this should presumably be the same, no?
> 
> No matter what the macro name is (let's call it N), it cannot be right 
> to do this:
> 
>     #ifndef N
>     # ifdef N
>     ... whatever ...
> 
> because the "whatever" cannot be reached. Unfortunately this is what 
> your patch does, with N being __ARG_NELTS.

Aaah.  I took what you had in your email and changed it to
__ARG_NELTS thinking you had a typo there because you had
suggested __ARG_NELTS before, and introduced a typo of my own.

> 
>> I see you have both __ARG_NELTS and
>> _ARG_NELTS_ in your Gnulib patch though I don't understand why.
> 
> It's for the same reason that regex.h already has both __restrict_arr 
> (the normal glibc practice) and  _Restrict_arr_ (an alternative spelling 
> used only in regex.h). This is because regex.h is used both in 
> /usr/include on GNU systems (where __restrict_arr is used) and in Gnulib 
> on non-GNU systems that may not have __restrict_arr.
> 
> regex.h does not simply #define __restrict_arr if it's not already 
> defined, because any such definition might collide with a later 
> GNU-system header that defines __restrict_arr in a different way (this 
> could happen if Gnulib regex.h is used on a older GNUish platform).
> 
> Obviously this is a messy situation and I'd like to clean it up, but as 
> I mentioned in my previous email any such cleanup something better left 
> for a later patch. In the meantime we should just continue existing 
> practice when we add a new macro like __ARG_NELTS.
> 
>> I plan to commit the last revision of the patch as is
> 
> Please don't do that. That patch still needs work. Instead, please start 
> with the patch I sent, and understand what it does. Unless I'm missing 
> something it should be able to go into glibc as-is (though you may need 
> to edit glibc files that are not in gnulib).

I think I'd prefer to let you do this.  It's far more complicated
and extensive than it needs to be to silence just the one warning.

Martin