From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 5591738582B1 for ; Tue, 8 Nov 2022 13:48:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5591738582B1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667915294; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=h46fbTYxYlrnJJVPcHwbx5WDU8ai1NwUwUcUAzAXo0o=; b=YFr1/1ojvSF5Z9L4dj6YWbySbplZijT5g9doVqcNElVzJ9MW3PSzoXk6P0Ho4tN0dzCafI L8824YdWdvHAUEsW+Jdit/u3pvlRcBX3Yetz5rqSEomZIjacXQ6URZEeIf55xKb+f44XiH RVp+rBxpsXlJLJhb3GkTVOCRiOsyg+g= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-675-AJbrNfsVPBaimCbj9N-BEw-1; Tue, 08 Nov 2022 08:48:12 -0500 X-MC-Unique: AJbrNfsVPBaimCbj9N-BEw-1 Received: by mail-io1-f72.google.com with SMTP id x23-20020a6b6a17000000b006d9ca4e35edso4513348iog.9 for ; Tue, 08 Nov 2022 05:48:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=h46fbTYxYlrnJJVPcHwbx5WDU8ai1NwUwUcUAzAXo0o=; b=0BTfzIwuueVXl4mTVRVvcMvGlAhBcLDofEIaK2NUdSMUtnkR/86QoA9A3vMPdLvu7F bA2Hgvv1h+3LgK6H+Ud6XhQtp9Hou4hgaHXFHgeTUhBsmTOcw7kUbwa+uNgtFJEKZ8E4 qGMpZrWNYuk9LGT+WvYNp0p93B75EsQhg+FYeSGd2DGr20MK5kAfxCVWimHpwABOEmMw hJZjPxvL+vQ1EFEIlgCFJxOCUypF+r3Gs4IfRo8Y6v2TgcupZ5wNNyWXEdZGje9dYtJt HA0Q5MbS2vy7x/9uoddP0Pp9Ym2SuQkYe+/nZew7bOhVoUDYcRZjMnkIZI4mwLPuofgK YoWA== X-Gm-Message-State: ACrzQf3cuzbQsi2gzi4TpUR2xGrtYL1XjPP/QQJmiZu1jEINEbs01WsL 4xusjbTwneV/OWocuedwXenb8bxAUyIuS+7lA5t3dLnfuwroXumszAb9jqIXjp6UA9emA4+y8J+ qr2YcUbttLd6EfkMPHGr4 X-Received: by 2002:a02:a04f:0:b0:375:444e:3dc0 with SMTP id f15-20020a02a04f000000b00375444e3dc0mr32569175jah.142.1667915291797; Tue, 08 Nov 2022 05:48:11 -0800 (PST) X-Google-Smtp-Source: AMsMyM608UUifC6BWpM5IeuyPN9YBOplOxIJVh/6tf3xhasZNTC5+XVi6GY1RLmHUHRLscnqmi3q9w== X-Received: by 2002:a02:a04f:0:b0:375:444e:3dc0 with SMTP id f15-20020a02a04f000000b00375444e3dc0mr32569160jah.142.1667915291421; Tue, 08 Nov 2022 05:48:11 -0800 (PST) Received: from [192.168.0.241] (192-0-145-146.cpe.teksavvy.com. [192.0.145.146]) by smtp.gmail.com with ESMTPSA id w127-20020a022a85000000b00363cce75bffsm3710808jaw.151.2022.11.08.05.48.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 08 Nov 2022 05:48:10 -0800 (PST) Message-ID: <83dbe5a2-26f8-b783-edbd-1b85f92b7a80@redhat.com> Date: Tue, 8 Nov 2022 08:48:09 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.0 Subject: Re: [PATCH] string: strtok, strtok_r should accept initial NULL subject (bug 16640) To: Florian Weimer , libc-alpha@sourceware.org References: <87bkpjm9d4.fsf@oldenburg.str.redhat.com> From: Carlos O'Donell Organization: Red Hat In-Reply-To: <87bkpjm9d4.fsf@oldenburg.str.redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/7/22 03:55, Florian Weimer via Libc-alpha wrote: > The BSD and musl implementations accept an initial NULL subject > argument. This also used to be support in glibc on some architectures > with custom assembler code. (1) Standards The ISO C standard pretty clearly says that the first call in the sequence has a non-null first argument. (2) Security >From a security stand point I do not want to see a change in behaviour where we go from failing quickly and immediately at the point we invoke undefined behaviour to potentially failing far from the point. We continue to move in this direction across all glibc APIs and we have previously discussed that we don't want to run with a potentially invalid state for any longer than required to shut the process down, and the shutdown itself should be as quick as possible. (3) Performance/Stability Terminating the process quickly reduces restart latency, which means the service can be restarted quickly when an error is encountered. (4) Compatibility I don't see any strong justification for why we would change the behaviour, other than compatibility with musl or certain BSDs. The compatibility of a undefined behaviour, poorly defined behaviour or implementation defined behaviour needs to be evaluated on the merits of the individual change. In this case I don't see a strong justification for exact compatibility in the "first call first argument NULL" case. I see a strong argument for harmonizing the behaviour and we did that in the glibc 2.26 release. Note that this behaviour is not implementation defined, but such behaviour would need to be documented. This is undefined behaviour. (5) Existing behaviour. The glibc implementation behaviour was not uniform across all architectures, and we fixed that in glibc 2.26. We have had this behaviour in place for 4 years across all the architectures, and even longer in the generic code. In summary: - I don't see a strong justification to change glibc. This gets a NACK from me. > Tested on i686-linux-gnu and x86_64-linux-gnu. > > --- > string/Makefile | 1 + > string/strtok_r.c | 6 +++++- > string/tst-strtok-null.c | 32 ++++++++++++++++++++++++++++++++ > 3 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/string/Makefile b/string/Makefile > index 938f528b8d..a1c6587376 100644 > --- a/string/Makefile > +++ b/string/Makefile > @@ -177,6 +177,7 @@ tests := \ > tst-strfry \ > tst-strlen \ > tst-strtok \ > + tst-strtok-null \ > tst-strtok_r \ > tst-strxfrm \ > tst-strxfrm2 \ > diff --git a/string/strtok_r.c b/string/strtok_r.c > index fd3a842c99..8342d2ac74 100644 > --- a/string/strtok_r.c > +++ b/string/strtok_r.c > @@ -44,7 +44,11 @@ __strtok_r (char *s, const char *delim, char **save_ptr) > char *end; > > if (s == NULL) > - s = *save_ptr; > + { > + if (*save_ptr == NULL) > + return NULL; > + s = *save_ptr; > + } > > if (*s == '\0') > { > diff --git a/string/tst-strtok-null.c b/string/tst-strtok-null.c > new file mode 100644 > index 0000000000..2cbc4e5fc4 > --- /dev/null > +++ b/string/tst-strtok-null.c > @@ -0,0 +1,32 @@ > +/* Check that strtok and strtok_r accept NULL for the initial subject string. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > + > +static int > +do_test (void) > +{ > + TEST_COMPARE_STRING (strtok (NULL, ","), NULL); > + char *save = NULL; > + TEST_COMPARE_STRING (strtok_r (NULL, ",", &save), NULL); > + return 0; > +} > + > +#include > > base-commit: 9cc9d61ee12f2f8620d8e0ea3c42af02bf07fe1e > -- Cheers, Carlos.