Re: [Cooker] strange problem with "hotplug" package

Re: [Cooker] strange problem with "hotplug" package

[Andrey Borzenkov]

> my situation below:
>  when i unplug my usb NIC and plug it back,
>  i found "ifplugd" running at my system.
>  and i have "MII_NOT_SUPPORTED=yes" at ifcfg-eth0,
>  so this souldn't be happened.
>  i trace the script /etc/hotplug/net.agent and
>  found that the "grep" command didn't work as expected.
>
>  i shrink the net.agent script as below:
>
>  #!/bin/bash
>  grep -q '^MII_NOT_SUPPORTED' /etc/sysconfig/network-scripts/ifcfg-eth0
>  echo $? > /tmp/grep_result
>
>  /tmp/grep_result is "0" when run script from command-line.(correct result)
>  /tmp/grep_result is "1" when i plug-in  my usb NIC. (wrong result!!)
>
>  i found if i add ">/dev/null" would solve the strange problem.
>  eg: fix first line to:
>  grep -q '^MII_NOT_SUPPORTED' /etc/sysconfig/network-scripts/ifcfg-eth0 >/dev/null

>  now /tmp/result would be "0" if i plug-in my usb NIC.
>  after fix the original net.agent, add ">/dev/null" to grep,
>  i solve my problem, ifplugd woun't start when i plug-in my usb NIC.
>  i found there are many "grep" at /etc/hotplug/* scripts.
>  maybe all these "grep" would suffer as mine.
>  i m curious what casue "grep" stop working without ">/dev/null".
>  anybody who can tell me ??

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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [Cooker] strange problem with "hotplug" package

[Stepan Kasal]

Hello to all people reading this,

On Tue, May 06, 2003 at 05:45:40PM +0400, Andrey Borzenkov wrote:
> bor@cooker% grep -q bor /etc/passwd <&- >&-
> grep: write error: Bad file descriptor
> bor@cooker% echo $?
> 1

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  <kasal@math.cas.cz>

	* 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.

--- grep-2.5.1.orig/src/grep.c	Tue Mar 26 16:54:12 2002
+++ grep-2.5.1/src/grep.c	Tue May  6 17:47:38 2003
@@ -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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [Cooker] strange problem with "hotplug" package

[Jim Meyering]

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 <kasal@math.cas.cz> wrote:
...
> +  /* don't close stdout for -q, consider ``grep -q pat <&- >&-'' */
> +  if (!exit_on_match)
> +    atexit(close_stdout);


-------------------------------------------------------
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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [Cooker] strange problem with "hotplug" package

[Stepan Kasal]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

Hello,

On Tue, May 06, 2003 at 10:12:20PM +0200, Jim Meyering wrote:
>   $ grep -q --help > /dev/full && echo ok
>   grep: write error: No space left on device
>   ok

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

> 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]

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.

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

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

[-- Attachment #2: grep-2.5.1-close_stdout2.patch --]
[-- Type: text/plain, Size: 3203 bytes --]

Thu May  8 14:08:57 CEST 2003  Stepan Kasal  <kasal@math.cas.cz>

	* 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
--- grep-2.5.1.orig/lib/closeout.c	Sun Mar  4 06:33:12 2001
+++ grep-2.5.1/lib/closeout.c	Thu May  8 13:50:14 2003
@@ -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
--- grep-2.5.1.orig/src/grep.c	Tue Mar 26 16:54:12 2002
+++ grep-2.5.1/src/grep.c	Thu May  8 14:05:27 2003
@@ -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 <bug-gnu-utils@gnu.org>.\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 ();

Re: [Cooker] strange problem with "hotplug" package

[Jim Meyering]

Thanks!

Stepan Kasal <kasal@math.cas.cz> wrote:
> @@ -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"));

A minor nit: it is customary to include the offending string
in such a diagnostic.  It's also nice to allow the user to specify
a prefix of the option argument: grep --color=al

To that end, you might want to use XARGMATCH as in e.g., coreutils' ls.c.
It does this for --time={atime, access, use, ctime, status}

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

With that and the corresponding declarations,
ls gives diagnostics like this:

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

You'd need lib/argmatch.[ch].


-------------------------------------------------------
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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel