This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] add attribute nonstring


On 11/12/2017 02:06 AM, Paul Eggert wrote:
Thanks for mentioning the problem with GCC 8. If the intent of
__attribute__((__nonstring__)) is to consistently mark char arrays that
are not necessarily null-terminated strings, then lots of data
structures in the tzdb code would need to be marked with this attribute.
For example, every member of tzfile.h's 'struct tzhead' would have the
attribute, because they are all char arrays that might not be
null-terminated.

Looking at the proposed glibc patch, it seems its intent is more modest.
All it's trying to do is to pacify GCC 8 so that GCC doesn't complain
about calls to strncpy and strncat. In that case, a simpler and cleaner
fix for tzdb is to stop using strncpy and strncat. (I suspect that the
main reason tzdb uses strncpy is that its code is so old that it
predates the widespread availability of memcpy.) So I installed the
attached patch into tzdb's zic.c and it should be part of the next tzdb
release.

Great!


Perhaps you can this idea elsewhere in the rest of the glibc patch too.
It would be a lot of work to consistently change glibc to use
__attribute__((__nonstring__)) for its documented purpose. If glibc can
avoid calling strncpy and strncat, it won't need __NONSTRING;

Replacing strncpy with memcpy works well in some case (such
the TZ_MAGIC macro).  In other cases the string pointed to by
the constant argument is unknown and can be shorter than the size
of the destination so in those the replacement would need to be
more involved.  I tried to make only minimal changes to suppress
the new warning to avoid excessive churn.  I also didn't want to
remove the originally intended and correct uses of strncpy (to
completely fill a buffer).

I have another GCC enhancement that I plan to submit this week
that points out uses of these non-string arrays with common
string functions like strlen.  With it, annotating more of
the non-string arrays with the attribute to help detect their
accidental misuses might be preferable to using memcpy.  I'll
leave that decision to the Glibc folks here.

 if not,
then I suggest changing this:

/* Describes a char array that is not necessarily a NUL-terminated
   string.  */
# define __NONSTRING __attribute__ ((__nonstring__))

to something more like this:

/* Describes a char array whose address can safely be passed as the first
   argument to strncpy and strncat, as the char array is not necessarily
   a NUL-terminated string.  */
# define __NONSTRING __attribute__ ((__nonstring__))

in sys/cdefs.h, so that the more-modest goal of the patch is clearer to
the reader.

I'll CC: this to tz@iana.org as a heads-up there.

That would make sense to me as well  I'll wait to submit an updated
patch until this and Joseph's style question have been settled.

Thanks
Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]