From patchwork Sun Nov 12 09:06:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 24204 Received: (qmail 3820 invoked by alias); 12 Nov 2017 09:06:48 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 3809 invoked by uid 89); 12 Nov 2017 09:06:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=goal, 8s, modest, H*Ad:D*iana.org X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH] add attribute nonstring To: Martin Sebor References: From: Paul Eggert Cc: GNU C Library , Time zone mailing list Message-ID: <85ee9ddd-0be5-04d3-7319-600bcf1eb8a6@cs.ucla.edu> Date: Sun, 12 Nov 2017 01:06:43 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: 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. 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; 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. From 641f2c48b10dec5ab6dd90f560b66e061aa660ba Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 12 Nov 2017 00:57:19 -0800 Subject: [PROPOSED] Port to GCC 8 -Wstringop-truncation Problem reported by Martin Sebor in: https://sourceware.org/ml/libc-alpha/2017-11/msg00336.html * NEWS: Mention this. * zic.c (writezone): Use memcpy, not strncpy. --- NEWS | 3 +++ zic.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 966f9b6..7881936 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,9 @@ Unreleased, experimental changes Diagnostics and commentary now distinguish UT from UTC more carefully; see theory.html for more information about UT vs UTC. + zic has been ported to GCC 8's -Wstringop-truncation option. + (Problem reported by Martin Sebor.) + Release 2017c - 2017-10-20 14:49:34 -0700 diff --git a/zic.c b/zic.c index 82e653d..e36301f 100644 --- a/zic.c +++ b/zic.c @@ -1960,7 +1960,7 @@ writezone(const char *const name, const char *const string, char version) } #define DO(field) fwrite(tzh.field, sizeof tzh.field, 1, fp) tzh = tzh0; - strncpy(tzh.tzh_magic, TZ_MAGIC, sizeof tzh.tzh_magic); + memcpy(tzh.tzh_magic, TZ_MAGIC, sizeof tzh.tzh_magic); tzh.tzh_version[0] = version; convert(thistypecnt, tzh.tzh_ttisgmtcnt); convert(thistypecnt, tzh.tzh_ttisstdcnt);