Discussion:
Unneeded compiler warning in glob.c
(too old to reply)
Tim Rühsen
2017-12-13 09:38:39 UTC
Permalink
Raw Message
Here is a patch to silence this warning:

glob.c: In function 'rpl_glob':
glob.c:618:64: warning: pointer of type 'void *' used in arithmetic
[-Wpointer-arith]
err = getpwnam_r (s.data, &pwbuf, s.data + ssize,


With Best Regards, Tim
Paul Eggert
2017-12-16 01:30:52 UTC
Permalink
Raw Message
On 12/13/2017 01:38 AM, Tim RÃŒhsen wrote:
> Here is a patch to silence this warning:
>
> glob.c: In function 'rpl_glob':
> glob.c:618:64: warning: pointer of type 'void *' used in arithmetic
> [-Wpointer-arith]
> err = getpwnam_r (s.data, &pwbuf, s.data + ssize,
>
>
> With Best Regards, Tim
>
Thanks, I tweaked it slightly to avoid inserting a cast (as it's better
to avoid casts when it's easy), and installed the attached patch into
Gnulib, in your name. I'll CC: Adhemerval as this should go into glibc
too, at some point.
Bruno Haible
2017-12-16 03:12:18 UTC
Permalink
Raw Message
Paul Eggert wrote:
> On 12/13/2017 01:38 AM, Tim Rühsen wrote:
> > Here is a patch to silence this warning:
> >
> > glob.c: In function 'rpl_glob':
> > glob.c:618:64: warning: pointer of type 'void *' used in arithmetic
> > [-Wpointer-arith]
> > err = getpwnam_r (s.data, &pwbuf, s.data + ssize,
> >
> >
> > With Best Regards, Tim
> >
> Thanks, I tweaked it slightly to avoid inserting a cast (as it's better
> to avoid casts when it's easy), and installed the attached patch into
> Gnulib, in your name.

... and I've added the ChangeLog entry for it.

But, when we look at the definition

struct scratch_buffer {
void *data; /* Pointer to the beginning of the scratch area. */
size_t length; /* Allocated space at the data pointer, in bytes. */
...
};

it seems quite natural to do pointer arithmetic on 'data'. Namely, this will
happen each time a scratch_buffer gets booked by several (not just one)
pieces of memory.

Therefore I would suggest to change the type of the 'data' field to 'char *'
in gnulib. I know that in glibc this is not needed, because glibc assumes GCC
and GCC supports 'void *' arithmetic as an extension.

Rationale: 'scratch_buffer' is a module of its own right in gnulib, and
may get more uses, in order to avoid stack overflows.

Bruno
Paul Eggert
2017-12-16 03:53:13 UTC
Permalink
Raw Message
On 12/15/2017 07:12 PM, Bruno Haible wrote:
> Therefore I would suggest to change the type of the 'data' field to 'char *'
> in gnulib. I know that in glibc this is not needed, because glibc assumes GCC
> and GCC supports 'void *' arithmetic as an extension.
>
> Rationale: 'scratch_buffer' is a module of its own right in gnulib, and
> may get more uses, in order to avoid stack overflows.

I think I'd rather leave it alone. glibc is moving to a more-general
module that addresses this problem in a better way, and I'd rather we
spend our limited resources doing that than forking.
Bruno Haible
2017-12-16 05:03:13 UTC
Permalink
Raw Message
I wrote:
> struct scratch_buffer {
> void *data; /* Pointer to the beginning of the scratch area. */
> size_t length; /* Allocated space at the data pointer, in bytes. */
> ...
> };
>
> it seems quite natural to do pointer arithmetic on 'data'. Namely, this will
> happen each time a scratch_buffer gets booked by several (not just one)
> pieces of memory.

Oops. scratch_buffer is not appropriate for this kind of use, as it does
not contain a 'size_t used;' field (like linebuffer does).

I was thinking at a combination between scratch_buffer and the 'alloca_used'
logic from glob.c. But this is beyond what scratch_buffer does.

Bruno
Adhemerval Zanella
2017-12-19 12:53:26 UTC
Permalink
Raw Message
On 15/12/2017 23:30, Paul Eggert wrote:
> On 12/13/2017 01:38 AM, Tim Rühsen wrote:
>> Here is a patch to silence this warning:
>>
>> glob.c: In function 'rpl_glob':
>> glob.c:618:64: warning: pointer of type 'void *' used in arithmetic
>> [-Wpointer-arith]
>>                         err = getpwnam_r (s.data, &pwbuf, s.data + ssize,
>>
>>
>> With Best Regards, Tim
>>
> Thanks, I tweaked it slightly to avoid inserting a cast (as it's better to avoid casts when it's easy), and installed the attached patch into Gnulib, in your name. I'll CC: Adhemerval as this should go into glibc too, at some point.
>

Thanks for the heads up, I will sync it on glibc side.
Loading...