Hello Berny and all,

On 2018-12-19 7:26 a.m., Bernhard Voelker wrote:
On 12/19/18 9:10 AM, Assaf Gordon wrote:
I noticed this small issue in base64 (seems the code is from
the original commit in 2006 when switching to git).

Nice catch!

Thanks for the quick reviewing!


Comments:
* a test may be good, wouldn't it?

Test is always a good idea, attached updated patch with tests.


* 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 :)

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

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?



regards,
 - assaf



>From 8021dd7c70f958fa42f971422ab2c9a61c4ea0a9 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email protected]>
Date: Wed, 19 Dec 2018 01:02:32 -0700
Subject: [PATCH] base64: fix 'extra operand' error message

In the following invocation, 'a' is the input file, and 'b' is the extra
operand:

  $ base64 a b

Report 'b' in the error message instead of 'a':

  $ base64 a b
  base64: extra operand 'b'

* src/base64.c (main): If there is more than one non-option operand,
report the second one (assuming the first is a the input file name).
* tests/misc/base64.pl: Add tests.
---
 src/base64.c         | 2 +-
 tests/misc/base64.pl | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/base64.c b/src/base64.c
index 1f5c09aae..3ee5b3388 100644
--- a/src/base64.c
+++ b/src/base64.c
@@ -310,7 +310,7 @@ main (int argc, char **argv)
 
   if (argc - optind > 1)
     {
-      error (0, 0, _("extra operand %s"), quote (argv[optind]));
+      error (0, 0, _("extra operand %s"), quote (argv[optind+1]));
       usage (EXIT_FAILURE);
     }
 
diff --git a/tests/misc/base64.pl b/tests/misc/base64.pl
index 0eb8cf498..a544558cb 100755
--- a/tests/misc/base64.pl
+++ b/tests/misc/base64.pl
@@ -62,6 +62,7 @@ my @Tests;
 sub gen_tests($)
 {
   my ($prog) = @_;
+  my $try_help = "Try '$prog --help' for more information.\n";
   @Tests=
     (
      ['empty', {IN=>''}, {OUT=>""}],
@@ -113,6 +114,12 @@ sub gen_tests($)
      ['b4k-1',   '--decode', {IN=>$a3k_nl[1]}, {OUT=>'a' x (3072+0)}],
      ['b4k-2',   '--decode', {IN=>$a3k_nl[2]}, {OUT=>'a' x (3072+0)}],
      ['b4k-3',   '--decode', {IN=>$a3k_nl[3]}, {OUT=>'a' x (3072+0)}],
+
+     ['ext-op1', 'a b',       {IN=>''}, {EXIT=>1},
+      {ERR => "$prog: extra operand 'b'\n" . $try_help}],
+     # Again, with more option arguments
+     ['ext-op2', '-di --wrap=40 a b',       {IN=>''}, {EXIT=>1},
+      {ERR => "$prog: extra operand 'b'\n" . $try_help}],
     );
 
   if ($prog eq "base64")
-- 
2.11.0

Reply via email to