On 12/20/18 4:46 AM, Assaf Gordon wrote: > On 2018-12-19 7:26 a.m., Bernhard Voelker wrote: >> On 12/19/18 9:10 AM, Assaf Gordon wrote: >> Comments: >> * a test may be good, wouldn't it? > > Test is always a good idea, attached updated patch with tests.
Thanks, the test looks good. >> * base32 is affected, too. It's the same source, but the fix should >> be mentioned for both programs. >> * this is a user-visible change, therefore it deserves a NEWS entry. > > This is a tiny change in the error message - not sure it warrants > a NEWS entry - it went 12 years with no one noticing or complaining :) Still, I think a user-visible fix, so I'd squash in something like: --- a/NEWS +++ b/NEWS @@ -6,0 +7,4 @@ GNU coreutils NEWS -*- outline -*- + In 'base64 a b', and likewise for base32, the tool now correctly + diagnoses 'b' as the extra operand, not 'a'. + [bug introduced in coreutils-5.3.0] + > As for base32, I see two commits for base64 (that happened after adding > base32), and they don't mention base32 despite affecting it as well: > > * df3b9120b base64: no longer support hex or oct --wrap params > * a8cc9ce3f base64: use stricter validation on wrap column True, but these commits where immediately done after introducing base32, i.e., in the same CU version. > I don't have strong feeling either way (adding NEWS and mention base32 > or not), happy to send another patch with those if you think they are > needed. should we support git commit message with "base64/32" prefix? We had quite a long time referencing the sha*sum tools separately, and only switched to using that wildcard when more utilities came in, so IMO it's good enough for now to mention them separately; thus I suggest: - Subject: [PATCH] base64: fix 'extra operand' error message + Subject: [PATCH] base32,base64: fix 'extra operand' error message FWIW: 'src/base64.c' should mention 'base32' in its title line. Maybe in a separate patch (below)? Thanks & have a nice day, Berny >From 0442eb3421eef627b7c87598d5db4ce7c08a1c43 Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <[email protected]> Date: Fri, 21 Dec 2018 08:31:00 +0100 Subject: [PATCH] maint: mention base32 in the title line of common base64.c * src/base64.c: Do the above, and remove a redundant comment. --- src/base64.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/base64.c b/src/base64.c index 3ee5b3388..ef315cfe3 100644 --- a/src/base64.c +++ b/src/base64.c @@ -1,8 +1,6 @@ -/* Base64 encode/decode strings or files. +/* Base64, base32 encode/decode strings or files. Copyright (C) 2004-2018 Free Software Foundation, Inc. - This file is part of Base64. - This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or -- 2.19.2
