Re: [PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

On Fri, Jul 05, 2024 at 10:39:52PM GMT, Jonathan Wakely wrote:
> On Fri, 5 Jul 2024 at 21:55, Alejandro Colomar <alx@xxxxxxxxxx> wrote:
> >
> > On Fri, Jul 05, 2024 at 09:28:46PM GMT, Jonathan Wakely wrote:
> > > > If we marked endptr as "write_only" (which it might already
> > > > be) then a future warning mechanism for -Wrestrict could
> > > > ignore the content of *endptr.
> > >
> > >
> > > That seems more useful. Add semantic information instead of taking it
> > > away. If the concern is a hypothetical future compiler warning that
> > > would give false positives for perfectly valid uses of strtol, then
> > > the problem is the compiler warning, not strtol. If additional
> > > information can be added to avoid the false positives (and also
> > > possibly optimize the code better), then that wouldn't require a
> > > change to the standard motivated by a hypothetical compiler warning.
> >
> > Let me be a little bit sarcastic.
> >
> > If so, let's take down -Wrestrict at all, because it triggers false
> > positives at the same rate.  How is it even in -Wall and not just
> > -Wextra?
> >
> > Here's a false positive:
> >
> >         $ cat d.c
> >         int is_same_pointer(const char *restrict ca, char *restrict a)
> >         {
> >                 return ca == a;
> 
> This is a strawman argument, because all your example functions have
> been pretty useless and/or not good uses of restrict.
> 
> Yes, if you put restrict on functions where you don't ever access any
> objects through the pointers, the restrict qualifiers are misleading

That's precisely the case with strtol(3): it doesn't access any objects
through *endptr, and so that pointer need not be restrict.

Then, nptr is a read-only pointer, so is doesn't matter either if it's
accessed or not.

Let's say we add as many attributes as possible to strtol(3):

[[gnu::access(read_only, 1)]]
[[gnu::access(write_only, 1)]]
[[gnu::leaf]]
[[gnu::nothrow]]
[[gnu::null_terminated_string_arg(1)]]
 // [[gnu::access(none, *1)]]
long
alx_strtol(const char *nptr, char **_Nullable restrict endp, int base);

Let's say we could mark *endptr as a 'access(none)' pointer, since it's
not accessed.  Let's say we do that with [[gnu::access(none, *1)]].

Then, do you think the information of that prototype is any different
than a prototype with restrict on the remaining pointers?

[[gnu::access(read_only, 1)]]
[[gnu::access(write_only, 1)]]
[[gnu::leaf]]
[[gnu::nothrow]]
[[gnu::null_terminated_string_arg(1)]]
 // [[gnu::access(none, *1)]]
long
alx_strtol(const char *restrict nptr,
           char *restrict *_Nullable restrict endp, int base);

I don't think so.  Since *endptr is access(none), it certainly cannot
access nptr, and thus the qualifier on nptr is superfluous.

And even without the hypothetical [[gnu::access(none, *1)]]:

-  The callee doesn't care about restrict, because it doesn't access
   any objects via *endptr, so it certainly knows that nptr can be read
   without any concerns about optimization.

-  The caller can't know if strtol(3) accesses *endptr, or nptr, and so
   it can't optimize.  Unless it passes an uninitialized value in
   *endptr, which means the caller knows for sure that nptr won't be
   written, regardless of restrict on it or not.

Please, describe what's the information you think is being added by
having restrict on nptr, on how it would be lost if we remove it.

Cheers,
Alex

> and so compilers might give bad warnings for your bad API.

-- 
<https://www.alejandro-colomar.es/>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux