Collin Funk wrote:
> At least to me, this looks a bit awkward:
>
> ATTRIBUTE_ACCESS ((none, 1))
Since 'none' may be defined as a C macro, the user would have to write
ATTRIBUTE_ACCESS ((__none__, 1))
which is even more awkward.
> >> +# define _GL_ATTRIBUTE_ACCESS_READ_ONLY(args) __attribute__ ((__access__
> >> (__read_only__, args)))
> >> +# define _GL_ATTRIBUTE_ACCESS_WRITE_ONLY(args) __attribute__
> >> ((__access__ (__write_only__, args)))
> >> +# define _GL_ATTRIBUTE_ACCESS_READ_WRITE(args) __attribute__
> >> ((__access__ (__read_write__, args)))
> >> +# if _GL_GNUC_PREREQ (11, 1)
> >> +# define _GL_ATTRIBUTE_ACCESS_NONE(args) __attribute__ ((__access__
> >> (__none__, args)))
> >
> > Can you try to keep line lengths at <= 80 columns?
>
> I was going to because that was my preference as well, but I noticed
> some other lines were longer than 80 lines in that file
Admitted, there are a couple of bad precedents in that file, especially
in #if conditions.
> > Regarding the comments: Is the "optional size S" measured in bytes or in
> > pointer
> > units? I.e. can the attribute be used e.g. on the wmemcpy function, whose
> > argument designates wchar_t elements, not bytes? The GCC docs don't state
> > it,
> > therefore it would be good to state it here.
>
> It is mentioned a few paragraphs in:
>
> The optional size-index denotes the position of a function argument
> of integer type that specifies the maximum size of the access. The
> size is the number of elements of the type referenced by ref-index,
> or the number of bytes when the pointer type is void*.
>
> So, in that example you would use the number of wchar_t elements.
Thanks. Apparently this text needs careful reading. Your new comments are
more concise and down-to-earth.
> I attached a v2 patch.
Looks good to me, except for one comment in attribute.h:
+/* _GL_ATTRIBUTE_ACCESS_READ_ONLY ((N, S)) declares that the Nth argument of
+ the function is used to read the referenced object but not to write to it.
+ _GL_ATTRIBUTE_ACCESS_WRITE_ONLY ((N, S)) declares that the Nth argument of
+ the function is used to write the referenced object but not to read from it.
+ _GL_ATTRIBUTE_ACCESS_READ_WRITE ((N, S)) declares that the Nth argument of
+ the function is used to both read from and write to the referenced object.
+ _GL_ATTRIBUTE_ACCESS_NONE ((N, S)) declares that the Nth argument of the
+ function is not used to access the referenced object at all.
+
+ Each of the _GL_ATTRIBUTE_ACCESS_* macros accepts an optional size index S,
+ which specifies the position of the argument indicating the maximum size of
+ the access, in elements. The size of each element is determined by the type
+ of the Nth argument, or is 1 if that argument is a void pointer. The memory
+ pointed to by the sized object is not required to be initialized. These
+ macros specify how the object being pointed to can be accessed, not that the
+ access will happen. They also do not imply whether or not the pointer is or
+ can be NULL. This information can be specified by combining the
+ _GL_ATTRIBUTE_ACCESS_* macros with _GL_ATTRIBUTE_NONNULL or
+ _GL_ATTRIBUTE_NONNULL_IF_NONZERO.
+ */
Here s/_GL_ATTRIBUTE_/ATTRIBUTE_/g
because this comment is supposed to document the ATTRIBUTE_* macros.
> I generally
> avoid making large changes, as you did when reducing variable scope
Sure. As you prefer. The main argument against large changes are not
typos (because they can happen in small changes as well, with the same
percentage), but that one has overlooked something. Such as when reducing
the variable scope, I had overlooked that
label: int foo;
is not valid in C versions before ISO C23.
Whereas the main argument for large changes is consistency.
Bruno
