Re: [PATCH] base64: fix 'extra operand' error message

[Bernhard Voelker]

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 <address@hidden>
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