From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11903 Path: news.gmane.org!.POSTED!not-for-mail From: Bartosz Brachaczek Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] handle whitespace before %% in scanf Date: Mon, 4 Sep 2017 23:56:41 +0200 Message-ID: <571dc49e-fea9-d31b-431b-9c90efb2cf64@gmail.com> References: <20170709210018.16369-1-b.brachaczek@gmail.com> <20170711012039.GO1627@brightrain.aerifal.cx> <20170904205933.GN1627@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------E79A820FF2E655E71FE8AEB3" X-Trace: blaine.gmane.org 1504562225 4919 195.159.176.226 (4 Sep 2017 21:57:05 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 4 Sep 2017 21:57:05 +0000 (UTC) User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 To: musl@lists.openwall.com Original-X-From: musl-return-11916-gllmg-musl=m.gmane.org@lists.openwall.com Mon Sep 04 23:56:51 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1dozMQ-0000bz-OF for gllmg-musl@m.gmane.org; Mon, 04 Sep 2017 23:56:50 +0200 Original-Received: (qmail 9818 invoked by uid 550); 4 Sep 2017 21:56:56 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 9800 invoked from network); 4 Sep 2017 21:56:55 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language; bh=D96MncB7sRR40UoMtMnnNtvzf2zasi1CuCKcZ4Qmv90=; b=Vv7jOd1iBTLrsKI/Hg/agX+NkByQjOG9z5cnoRn/7sOXtpDwL4j+ZBWQ+12YcP682q S3gT7ODILta+h6B7QmQEJuOe82pBLpoBw1y0GvlrKL/8XlqzDw1ksgT6ALy3DI3JfppO rYn16oBd7IdVrs15DeoNDBvl68X0jTUCNI8/6COQBSkr28WY9NQNsKJLG94ZZWZ6JQTK Bp2rGxI7mvMtlcyj/J/qK90KnSqpnjXaeaO3nUrKdf/gkxDmw/2iLlsVRU3Jh2yndZMq uR2/e05QQdNLjNDyv9B4ITDZ37hAu/RoIF4YyBzXqji3UFTalGJ8JAu8bydJgj0nCT+y mGnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=D96MncB7sRR40UoMtMnnNtvzf2zasi1CuCKcZ4Qmv90=; b=snXXZ6wN6Rd2pXUcsc9GBQLuNhN/B/UXRN5kWbkPLcIpxfAcE6rFkCA1e1BuQxRJha 6Wkh4lj//rKt3lzrsvlm1s84LbCgLAZhHCr19dKhLpOgoW1926IzIw/0+bArr6YbZeua mCqU2jkiwnmyFn2/4mdyKzo5kQtjB80g9W0PFcNMXWIhcFBVTEm0j4aV7OthgnULKJQl jLf1Y+6XGAX0BTovbIpHzy1rCPOcjHqvTDjqUKBZchJijKO/OvXtPCbxvVZQ0t0g4MKv 4H2IMvY7IQoNx8gCnw8LYBetP0f5sE7yWCmj4WCJbll6Kk4x0zNK+TWY5YKj+unYfPCp iYUA== X-Gm-Message-State: AHPjjUgwg/sGm/DVTOrr5ztCM3WLAmxWu51bF0eyGHZk4wohxZ/QXhCQ a7yvmwM+7Me/+XUCQS0= X-Google-Smtp-Source: ADKCNb7gkLmEz+Ogrc1/tmqAKK2OjigdZ3aGXhHm8WJIniELkLkZexvyFwsqIEBanDKHsEk5a2FwLQ== X-Received: by 10.28.180.86 with SMTP id d83mr1391679wmf.172.1504562203797; Mon, 04 Sep 2017 14:56:43 -0700 (PDT) In-Reply-To: <20170904205933.GN1627@brightrain.aerifal.cx> Content-Language: en-US Xref: news.gmane.org gmane.linux.lib.musl.general:11903 Archived-At: This is a multi-part message in MIME format. --------------E79A820FF2E655E71FE8AEB3 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 9/4/2017 10:59 PM, Rich Felker wrote: > On Mon, Jul 10, 2017 at 09:20:39PM -0400, Rich Felker wrote: >> On Sun, Jul 09, 2017 at 11:00:18PM +0200, Bartosz Brachaczek wrote: >>> this is mandated by C and POSIX standards and is in accordance with >>> glibc behavior. >>> --- >>> src/stdio/vfscanf.c | 10 +++++++--- >>> src/stdio/vfwscanf.c | 8 ++++++-- >>> 2 files changed, 13 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/stdio/vfscanf.c b/src/stdio/vfscanf.c >>> index d4d2454b..9e030fc4 100644 >>> --- a/src/stdio/vfscanf.c >>> +++ b/src/stdio/vfscanf.c >>> @@ -89,15 +89,19 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap) >>> continue; >>> } >>> if (*p != '%' || p[1] == '%') { >>> - p += *p=='%'; >>> shlim(f, 0); >>> - c = shgetc(f); >>> + if (*p == '%') { >>> + p++; >>> + while (isspace((c=shgetc(f)))); >>> + } else { >>> + c = shgetc(f); >>> + } >>> if (c!=*p) { >>> shunget(f); >>> if (c<0) goto input_fail; >>> goto match_fail; >>> } >>> - pos++; >>> + pos += shcnt(f); >>> continue; >>> } >> >> Assuming your interpretation is correct, I have no objection to going >> forward with the change, but I don't think this is the right way to do >> it. The only reason %% was handled in the code that handles literal >> characters is because I assumed it behaves like one, but if it >> doesn't, it should just be handled as a format specifier that consumes >> space where it can use the existing code that does that, rather than >> complicting the code for literals and adding a duplicate of the >> space-skipping code to it. > > I tried going forward with the idea I proposed, but it looks like it's > actually more invasive: in addition to adding the final case to > actually handle '%', it adds a new case where a conversion specifier > does not consume a variadic input, and a new case where width is > forced to 1 and modifier flags and explicit widths are rejected. > > As such I think your patch as originally submitted is probably the > best approach. Sorry for the delay in reviewing and accepting it. Oh, that's perfect, thanks. Sorry I didn't get to responding to your request. I originally tried both approaches and chose the one that had smaller impact on code size in vfscanf.o. While at it, you might want to have a look at another trivial patch for vfwscanf I submitted: http://www.openwall.com/lists/musl/2017/07/09/5 If anything, it avoids confusion for people reading the code. I'm attaching a version of this patch with enough context lines for inline review. --------------E79A820FF2E655E71FE8AEB3 Content-Type: text/plain; charset=UTF-8; name="0001-slightly-simplify-a-condition-in-vfwscanf.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="0001-slightly-simplify-a-condition-in-vfwscanf.patch" RnJvbSA1OTA5YzhhOWI4N2RiNTcyODUxMjJjODczNjg0NTc1NThiZWJmM2I2IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBCYXJ0b3N6IEJyYWNoYWN6ZWsgPGIuYnJhY2hhY3pl a0BnbWFpbC5jb20+CkRhdGU6IFN1biwgOSBKdWwgMjAxNyAyMjoyNTo1NCArMDIwMApTdWJq ZWN0OiBbUEFUQ0hdIHNsaWdodGx5IHNpbXBsaWZ5IGEgY29uZGl0aW9uIGluIHZmd3NjYW5m Cgp0aGUgT1IgaXMgbm90IG5lZWRlZCBzaW5jZSBjb21taXQKZGU4MGVhOWYxYzI4MjFjYmI0 MjA1NTMzYjg2ZDVkMTdmOWU4ZDM3Ni4KLS0tCiBzcmMvc3RkaW8vdmZ3c2NhbmYuYyB8IDIg Ky0KIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQoKZGlm ZiAtLWdpdCBhL3NyYy9zdGRpby92ZndzY2FuZi5jIGIvc3JjL3N0ZGlvL3Zmd3NjYW5mLmMK aW5kZXggMWViYzVjZWYuLmFkOGUyYjlhIDEwMDY0NAotLS0gYS9zcmMvc3RkaW8vdmZ3c2Nh bmYuYworKysgYi9zcmMvc3RkaW8vdmZ3c2NhbmYuYwpAQCAtMTg0LDIxICsxODQsMjEgQEAg aW50IHZmd3NjYW5mKEZJTEUgKnJlc3RyaWN0IGYsIGNvbnN0IHdjaGFyX3QgKnJlc3RyaWN0 IGZtdCwgdmFfbGlzdCBhcCkKIAogCQl0ID0gKnA7CiAKIAkJLyogVHJhbnNmb3JtIFMsQyAt PiBscyxsYyAqLwogCQlpZiAoKHQmMHgyZik9PTMpIHsKIAkJCXNpemUgPSBTSVpFX2w7CiAJ CQl0IHw9IDMyOwogCQl9CiAKIAkJaWYgKHQgIT0gJ24nKSB7Ci0JCQlpZiAodCAhPSAnWycg JiYgKHR8MzIpICE9ICdjJykKKwkJCWlmICh0ICE9ICdbJyAmJiB0ICE9ICdjJykKIAkJCQl3 aGlsZSAoaXN3c3BhY2UoKGM9Z2V0d2MoZikpKSkgcG9zKys7CiAJCQllbHNlCiAJCQkJYz1n ZXR3YyhmKTsKIAkJCWlmIChjIDwgMCkgZ290byBpbnB1dF9mYWlsOwogCQkJdW5nZXR3Yyhj LCBmKTsKIAkJfQogCiAJCXN3aXRjaCAodCkgewogCQljYXNlICduJzoKIAkJCXN0b3JlX2lu dChkZXN0LCBzaXplLCBwb3MpOwotLSAKMi4xNC4xCgo= --------------E79A820FF2E655E71FE8AEB3--