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 Netto X-Patchwork-Id: 79147 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp3045434qge; Tue, 25 Oct 2016 03:59:07 -0700 (PDT) X-Received: by 10.99.159.17 with SMTP id g17mr32007878pge.149.1477393147010; Tue, 25 Oct 2016 03:59:07 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id cm3si16726554pad.308.2016.10.25.03.59.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 25 Oct 2016 03:59:06 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-74051-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-74051-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-74051-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; q=dns; s= default; b=e68poBDhPdpe7x8iIidprUsBHSOrlSn/AiLK+RpXEzEYDJHpHhNgl o4YNMRWtjqpJpejaBvcQSoPGdkxBPzcXV0HY4uxVOKhD3m0vN1BMMcBvMyyz8wUk uPGLcLkcVS5SX4vE/LC990Re2rMYsOou4Al1lWUcT1qMkHpTbRjQZ4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; s=default; bh=PfmlofIzWJKaMKBHv6XaJLX+9W0=; b=lepaGeNw12c0wFF2MsKyKwSVBcLJ jjfQkFNXtdlHG3k8IMK6HOrKhW0vpGhX9aTlPWHR0k9rfyw+65L31pYvc5NOAmeM Vuasbqtx6a6YK1uh5mJCnxWYtP81INKIbWXwckjd6UcLsC/ex4oZowmHv9oG8nQq 3AfqhSZbrVrflBc= 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(-) -- 2.7.4 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"