bug#6328: coreutils-7.5 and newer give sometimes spurious warnings on -r for --file being obsolete [take2]

[Jim Meyering]

Robin H. Johnson wrote:
> Hi,
>
> I'm emailing you as you introduced the original breakage per the
> coreutils ChangeLog.

It's best to send coreutils-related mail to bug-coreutils (Cc'd, now)
or address@hidden, rather than directly to me.

> Filed downstream at:
> http://bugs.gentoo.org/show_bug.cgi?id=322421
>
> If you use the short version "-r" for a reference file, the value of
> longindex is not trustable (as per all getopt_long calls), and so
> sometimes the warning triggers, and sometimes it doesn't.
>
> Example:
> # touch -r ./marker-tar marker-newest-file
> warning: the --file option is obsolete; use --reference
>
> Minimal testcase attached.
>
> Notice that longindex is not trustable with short option -r, and in the
> original src/touch.c, this can spuriously give the --file obsolete
> warning.
>
> Test output:
> ======
> CMD ./getopt_long_test -f foo bar
> opt c=f optarg=NULL long_idx=-1(INVALID)
> arg foo
> arg bar
> CMD ./getopt_long_test --file foo bar
> opt c=r optarg=foo long_idx=1 lo[idx].val=r lo[idx].name=file
> arg bar
> CMD ./getopt_long_test -r foo bar
> opt c=r optarg=foo long_idx=-1(INVALID)
> arg bar
> CMD ./getopt_long_test --reference foo bar
> opt c=r optarg=foo long_idx=2 lo[idx].val=r lo[idx].name=reference
> arg bar
> ======
>
> Fix suggestion:
> - change the val field of the --file longoption to be 'f', and put the
>   handling for the obsolete case there.
> - always reset longindex to an invalid value after each iteration of the
>   getopt_long loop, and check it for being valid before using it.

Thank you for the report and diagnosis.  That was indeed a bug.
Since that --file option was slated for removal this year,
your report serves as a fine excuse to get rid of it, now,
15 years after I undocumented it ;-)

Patch below.

> Robin Hugh Johnson
> Gentoo Linux: Developer, Trustee & Infrastructure Lead
> E-Mail     : address@hidden
> GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85
>
> #include <stdio.h>
> #include <getopt.h>
> #include <sys/types.h>
> #include <assert.h>
>
> static struct option const longopts[] = {
>       {"file", required_argument, NULL, 'r'}, /* FIXME: remove --file in 2010 
> */
>       {"reference", required_argument, NULL, 'r'},
>       {NULL, 0, NULL, 0}
> };
>
> int main (int argc, char** argv) {
>       int long_idx;
>       int c;
>       while((c = getopt_long(argc, argv, "fr:", longopts, &long_idx)) != -1) {
>               switch(c) {
>                       case 'f':
>                       case 'r':
>                               printf("opt long_idx=%d actual_c=%c 
> expected_c=%c longname=%s arg=%s\n",
>                                         long_idx, c,
>                                         longopts[long_idx].val,
>                                         longopts[long_idx].name,
>                                         (optarg) ? optarg : "NULL");
>                               break;
>               }
>       }
>
>       for (; optind < argc; ++optind) {
>               printf("arg %s\n", argv[optind]);
>       }
>
> }

>From cdabca0e89baadb2b9141902d6b7a8f2f512362a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 2 Jun 2010 16:23:41 +0200
Subject: [PATCH] touch: remove support for --file=REF_FILE option

* src/touch.c (main): Remove support for deprecated long-named
--file option (alternate name for --reference (-r)).
That option was undocumented with the arrival of --reference, in
the 1995-10-29 commit, 8b92864e1dd58318bd0a434c5bd453b9c1ef5d08.
Since the 2009-02-09 commit, v7.0-172-ged85df4, use of --file
has elicited a warning.  Not only was this code due for removal,
but the long-name-use-detecting code was buggy in that it would
use a stale or uninitialized "long_idx", as reported by
Robin H. Johnson in http://bugs.gentoo.org/322421.
* NEWS (Changes in behavior): Mention it.
---
 NEWS        |    5 +++++
 src/touch.c |    8 +-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 7d3343b..46067b8 100644
--- a/NEWS
+++ b/NEWS
@@ -14,10 +14,15 @@ GNU coreutils NEWS                                    -*- 
outline -*-

   sort -g now uses long doubles for greater range and precision.

+  touch's --file option is no longer recognized.  Use --reference=F (-r)
+  instead.  --file has not been documented for 15 years, and its use has
+  elicited a warning since coreutils-7.1.
+
   truncate now supports setting file sizes relative to a reference file.
   Also errors are no longer suppressed for unsupported file types, and
   relative sizes are restricted to supported file types.

+
 * Noteworthy changes in release 8.5 (2010-04-23) [stable]

 ** Bug fixes
diff --git a/src/touch.c b/src/touch.c
index fd1bfb1..32d99e5 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -86,7 +86,6 @@ static struct option const longopts[] =
   {"time", required_argument, NULL, TIME_OPTION},
   {"no-create", no_argument, NULL, 'c'},
   {"date", required_argument, NULL, 'd'},
-  {"file", required_argument, NULL, 'r'}, /* FIXME: remove --file in 2010 */
   {"reference", required_argument, NULL, 'r'},
   {"no-dereference", no_argument, NULL, 'h'},
   {GETOPT_HELP_OPTION_DECL},
@@ -263,7 +262,6 @@ main (int argc, char **argv)
   bool date_set = false;
   bool ok = true;
   char const *flex_date = NULL;
-  int long_idx; /* FIXME: remove in 2010, when --file is removed */

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -276,7 +274,7 @@ main (int argc, char **argv)
   change_times = 0;
   no_create = use_ref = false;

-  while ((c = getopt_long (argc, argv, "acd:fhmr:t:", longopts, &long_idx)) != 
-1)
+  while ((c = getopt_long (argc, argv, "acd:fhmr:t:", longopts, NULL)) != -1)
     {
       switch (c)
         {
@@ -304,10 +302,6 @@ main (int argc, char **argv)
           break;

         case 'r':
-          if (long_idx == 3)
-            error (0, 0,
-                   _("warning: the --%s option is obsolete; use --reference"),
-                   longopts[long_idx].name);
           use_ref = true;
           ref_file = optarg;
           break;
--
1.7.1.359.g19d56