strange problem with "hotplug" package

bor@cooker% grep -q bor /etc/passwd <&- >&-
grep: write error: Bad file descriptor
bor@cooker% echo $?
1

exec_usermodehelper closes all file descriptors so grep barfs. Adding > /dev/null makes a valid stdout.

This is om Mandrake 9.0; you say it is on 9.1 ... well, this definitely makes hotplug life harder.

i wonder why grep would write to stdout with -q:

-q, --quiet, --silent
Quiet; do not write anything to standard output. Exit immedi- ately with zero status if any match is found, even if an error was detected. Also see the -s or --no-messages option.

bor@cooker% uname -a
Linux cooker.mow.siemens.ru 2.4.20-2mdk #1 Mon Dec 9 15:07:53 CET 2002 i686 unknown unknown GNU/Linux bor@cooker% rpm -q hotplug grep
hotplug-2002_04_01-2mdk
grep-2.5-5mdk

-andrey

-------------------------------------------------------
Enterprise Linux Forum Conference & Expo, June 4-6, 2003, Santa Clara The only event dedicated to issues related to Linux enterprise solutions www.enterpriselinuxforum.com

_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Hello to all people reading this,

the exit code is always correct. The only problem is the bogus error message.
So, we should not try to close(stdout) if -q was given.

Curiously enough, the problem doesn't appear with

grep -q bor /etc/passwd >&-

because the file /etc/passwd is opened with file descriptor 1, and it is closed on exit instead of stdout.
(With <&- >&-, /etc/passwd is fd 0 and the bug bites.)
Cause: if a match was found, we exit(0) immediately, without closing the open file. This optimization can be moved after the input file is closed, without any loss.

The patch attached to the end of this mail should fix both problems. (Apply to the 2.5.1 source tree.) It will appear in the next release.

Regards,
Stepan Kasal

Tue May 6 17:49:29 CEST 2003 Stepan Kasal <[email protected]>

* src/grep.c(main): Don't register atexit(close_stdout) if -q was given---no output will be written; there is also no need to use close_stdout_set_status().
* src/grep.c(grepbuf): move exit(0) ...
(grepfile): ... here, when the bufdesc is closed; this doesn't present any performance loss, done_on_match is 1 and ensures that we get out quickly.

@@ -722,8 +722,6 @@ grepbuf (char const *beg, char const *li
           outleft--;
 	  if (!outleft || done_on_match)
 	    {
-	      if (exit_on_match)
-		exit (0);
 	      after_last_match = bufoffset - (buflim - endp);
 	      return nlines;
 	    }

@@ -978,6 +976,9 @@ grepfile (char const *file, struct stats
 	    }
     }
 
+  if (!status && exit_on_match)
+	exit (0);
+
   return status;
 }
 

@@ -1348,8 +1349,6 @@ main (int argc, char **argv)
   textdomain (PACKAGE);
 #endif
 
-  atexit (close_stdout);
-
   prepend_default_options (getenv ("GREP_OPTIONS"), &argc, &argv);
 
   while ((opt = get_nondigit_option (argc, argv, &default_context)) != -1)

@@ -1524,7 +1523,6 @@ main (int argc, char **argv)
 
       case 'q':
 	exit_on_match = 1;
-	close_stdout_set_status(0);
 	break;
 
       case 'R':

@@ -1630,6 +1628,10 @@ main (int argc, char **argv)
 	break;
 
       }
+
+  /* don't close stdout for -q, consider ``grep -q pat <&- >&-'' */
+  if (!exit_on_match)
+    atexit(close_stdout);
 
   /* POSIX.2 says that -q overrides -l, which in turn overrides the
      other output options.  */

-------------------------------------------------------
Enterprise Linux Forum Conference & Expo, June 4-6, 2003, Santa Clara The only event dedicated to issues related to Linux enterprise solutions www.enterpriselinuxforum.com

_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Hi Stepan!

With your new version of grep, will `grep -q --help > /dev/full' fail? I was surprised to see that with 2.5.1, it does give a diagnostic, but doesn't actually fail:

  $ grep -q --help > /dev/full && echo ok
  grep: write error: No space left on device
  ok

While without -q, it works as I'd expect:

  $ grep --help > /dev/full && echo ok
  grep: write error: No space left on device
  [Exit 1]

Jim

P.S. Thanks for taking up the reins of GNU grep.

Stepan Kasal <[email protected]> wrote:
...

Hello,

thank you very much for pointing this out; I haven't thought about this.

No, I'd expect exit code 2 in both cases, not 1.

I have revised my previous patch; see the patch attached to this mail. (It's relative to plain 2.5.1.) Feedback welcome, of course.

Well I have to admit that I haven't done much yet. But I hope I'll get to it soon. ;-)

Have a nice day,
Stepan Kasal

Thu May 8 14:08:57 CEST 2003 Stepan Kasal <[email protected]>

* lib/closeout.c (default_exit_status): set to 2.
* src/grep.c (main): Don't register atexit(close_stdout) if -q was given---no output will be written; there is also no need to use close_stdout_set_status(). Move the atexit() call after all calls to usage(); usage() and --version call close_stdout() explicitely before exit.
(grepbuf): move exit(0) ...
(grepfile): ... here, when the bufdesc is closed; this doesn't present any performance loss, done_on_match is 1 and ensures that we get out of grepbuf() quickly.

diff -urpN grep-2.5.1.orig/lib/closeout.c grep-2.5.1/lib/closeout.c
@@ -47,7 +47,8 @@ extern int errno;
 #include "__fpending.h"
 #endif
 
-static int default_exit_status = EXIT_FAILURE;
+static int default_exit_status = /* EXIT_FAILURE */
+	2 /* error exit code for grep */;
 static const char *file_name;
 
 /* Set the value to be used for the exit status when close_stdout is called.

diff -urpN grep-2.5.1.orig/src/grep.c grep-2.5.1/src/grep.c
@@ -722,8 +722,6 @@ grepbuf (char const *beg, char const *li
           outleft--;
 	  if (!outleft || done_on_match)
 	    {
-	      if (exit_on_match)
-		exit (0);
 	      after_last_match = bufoffset - (buflim - endp);
 	      return nlines;
 	    }

@@ -978,6 +976,9 @@ grepfile (char const *file, struct stats
 	    }
     }
 
+  if (!status && exit_on_match)
+	exit (0);
+
   return status;
 }
 

@@ -1125,6 +1126,7 @@ two FILEs given, assume -h.  Exit status
 and 2 if trouble.\n"));
       printf (_("\nReport bugs to <[email protected]>.\n"));
     }
+  close_stdout ();
   exit (status);
 }
 

@@ -1348,8 +1350,6 @@ main (int argc, char **argv)
   textdomain (PACKAGE);
 #endif
 
-  atexit (close_stdout);
-
   prepend_default_options (getenv ("GREP_OPTIONS"), &argc, &argv);
 
   while ((opt = get_nondigit_option (argc, argv, &default_context)) != -1)

@@ -1524,7 +1524,6 @@ main (int argc, char **argv)
 
       case 'q':
 	exit_on_match = 1;
-	close_stdout_set_status(0);
 	break;
 
       case 'R':

@@ -1579,7 +1578,7 @@ main (int argc, char **argv)
                   !strcasecmp(optarg, "if-tty"))
             color_option = 2;
           else
-            show_help = 1;
+	    error (2, 0, _("unrecognized colour option"));
         } else
           color_option = 2;
         if(color_option == 2) {

@@ -1667,6 +1666,7 @@ Copyright 1988, 1992-1999, 2000, 2001 Fr
 This is free software; see the source for copying conditions. There is NO\n\
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n"));
       printf ("\n");
+      close_stdout ();
       exit (0);
     }
 

@@ -1693,6 +1693,10 @@ warranty; not even for MERCHANTABILITY o
       }
     else
       usage (2);
+
+  /* don't close stdout for -q, consider ``grep -q pat <&- >&-'' */
+  if (!exit_on_match)
+    atexit (close_stdout);
 
   if (!install_matcher (matcher) && !install_matcher ("default"))
     abort ();

Thanks!

	case TIME_OPTION:
	  time_type = XARGMATCH ("--time", optarg, time_args, time_types);
	  break;

    $ ls --time=z
    ls: invalid argument `z' for `--time'
    Valid arguments are:
      - `atime', `access', `use'
      - `ctime', `status'
    Try `ls --help' for more information.