From patchwork Tue Oct 25 10:58:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 16791 Received: (qmail 40444 invoked by alias); 25 Oct 2016 10:58:55 -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 39950 invoked by uid 89); 25 Oct 2016 10:58:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=aligns, 1.10, test_function, UD:test-skeleton.c X-HELO: mail-ua0-f172.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=RMwbD60NtWwU4GxCl7Ktk8BRB9e4L9GmntkchvJuwcY=; b=Io1IZhhE3uqwdvNE5FDOamlY9MP0fXvLlrV6G4IpOhPmDZE+g1C0HuKn+2Zjshia3d 3ZTF1/1QtOwyVmGQ4++6JtUSA3mE/b+79s2fCVhfDzEVg3Dpn9ogFFr6gViCfJu0hngk ufF897BGqNNeCfzdO29vuOougDbgg+y98IArY3W1CNXXTf7ha17q9j7FEbnlqoBF0exg iyNb4Jj59FBQF7GLgYJp76aod4mRpo5YuQnYAIIVcakMPvH3WN1C+zftOoRNJPY9fpV0 WCgXo9NXeEb23onSTu2yIJJFI7tynWrouEMX9lc2fPhVoQ4GuoMNsrAlIiQ/AZ3RPMQ1 NlNA== X-Gm-Message-State: ABUngvcfcydCZk/nS0Cu2aELRnt8AnCg6mrkWw6LzC7CIH7tK1ayh33vNEautWyUHxC/k6FW X-Received: by 10.159.39.230 with SMTP id b93mr3611018uab.87.1477393121544; Tue, 25 Oct 2016 03:58:41 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH] Fix undefined behaviour inconsistent for strtok Date: Tue, 25 Oct 2016 08:58:33 -0200 Message-Id: <1477393113-3845-1-git-send-email-adhemerval.zanella@linaro.org> Although not stated in any standard how strtok should return if you pass a null argument if the previous argument is also null, this patch changes the default implementation to follow this idea. The original bug report comment #1 states glibc code convention [6] should not allow it, however for this specific function its contract does not expect failure even if the returned is ignored (since it would be a no-op). Also, patch idea is more focuses on implementation portability , since it aligns glibc with other implementation that follows the same idea for strtok: - FreeBSD [1], OpenBSD [2], NetBSD [3]; - uclibc and uclibc-ng [4] - musl [5] I see little value to either assert on null input (as stated in comment 2 from original bug report), change both x86_64 and powerpc64le implementation to fault on such input, or to keep a different behavior compared to other libc implementations. Checked on x86_64, aarch64, and powerpc64le. * string/strtok.c (strtok): Return null is previous input is also null. * string/tst-strtok.c (do_test): Add more strtok coverage. [1] https://github.com/freebsd/freebsd/blob/386ddae58459341ec567604707805814a2128a57/lib/libc/string/strtok.c [2] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/string/strtok_r.c?rev=1.10&content-type=text/x-cvsweb-markup&only_with_tag=MAIN [3] https://github.com/openbsd/src/blob/5271000b44abe23907b73bbb3aa38ddf4a0bce08/lib/libc/string/strtok.c [4] http://www.uclibc-ng.org/browser/uclibc-ng/libc/string/strtok_r.c [5] https://git.musl-libc.org/cgit/musl/tree/src/string/strtok.c [6] https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers --- ChangeLog | 7 +++++++ string/strtok.c | 4 ++-- string/tst-strtok.c | 23 ++++++++++++----------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/string/strtok.c b/string/strtok.c index 7a4574d..5c4b309 100644 --- a/string/strtok.c +++ b/string/strtok.c @@ -40,8 +40,8 @@ STRTOK (char *s, const char *delim) { char *token; - if (s == NULL) - s = olds; + if ((s == NULL) && ((s = olds) == NULL)) + return NULL; /* Scan leading delimiters. */ s += strspn (s, delim); diff --git a/string/tst-strtok.c b/string/tst-strtok.c index 6fbef9f..d9180a4 100644 --- a/string/tst-strtok.c +++ b/string/tst-strtok.c @@ -2,25 +2,26 @@ #include #include +static int do_test (void); + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + static int do_test (void) { char buf[1] = { 0 }; int result = 0; + if (strtok (NULL, " ") != NULL) + FAIL_RET ("first strtok call did not return NULL"); + if (strtok (NULL, " ") != NULL) + FAIL_RET ("second strtok call did not return NULL"); + if (strtok (buf, " ") != NULL) - { - puts ("first strtok call did not return NULL"); - result = 1; - } + FAIL_RET ("third strtok call did not return NULL"); else if (strtok (NULL, " ") != NULL) - { - puts ("second strtok call did not return NULL"); - result = 1; - } + FAIL_RET ("forth strtok call did not return NULL"); return result; } - -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c"