Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Jim Meyering 
Paul Eggert wrote:
 Eric Blake e...@byu.net writes:
 I just checked POSIX, and this is entirely true - the exec*() family is
 allowed to inherit an ignored SIGCHLD, in deference to older systems like
 SysV; and wait()/waitpid() are allowed to fail with ECHILD if SIGCHLD is
 ignored.  But most systems these days explicitly set SIGCHLD to SIG_DFL on
 fork; are there still any modern systems where the explicit signal change
 is needed?

 I'd be a bit leery of assuming this.  It was such a pervasive problem
 back in the day.  I'd be a bit surprised if the problem were eliminated
 from all modern systems in all compilation modes.

 But in looking at the gnulib code for execute.c, I don't see any mention
 of this SIGCHLD anomaly, where wait/waitpid fail if SIGCHLD is ignored.
 On the other hand, gnulib's execute module uses waitid rather than
 waitpid; I guess that this choice of API is immune to the SysV behavior?

 Yes, I expect so.

 The simplest thing is to leave install.c alone.  Porting to mingw is low
 priority, surely.

[reviving an old thread...]

I was considering yet again the sole use of signal in install.c,

#ifdef USE_SIGNAL
  signal (SIGCHLD, SIG_DFL);
#endif

It is tempting to remove it, but I've just confirmed that we must
not, as long as the linux-2.6.9 kernel is still relevant.
[it's still in use in every RHEL 4.7 installation]

I confirmed that running with SIGCHLD ignored, and regardless
of which wait function it uses (wait, waitpid, or waitid), the
parent still fails to determine the child's exit status.  I.e.,
each of the wait functions does wait, but then returns -1/ECHILD
rather than the child's PID.

Darwin 8.11.0 still requires the signal call too, but there I merely
confirmed that coreutils' install/trap test would fail without the
signal call.

Which makes me think gnulib's wait-process.c will have to be changed
to deal with this.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Bruno Haible 
Jim Meyering wrote:
 [reviving an old thread...]

This was a reply to
http://lists.gnu.org/archive/html/bug-coreutils/2008-10/msg00216.html,
for those who - like me - lost the context after 4 months.

 I confirmed that running with SIGCHLD ignored, and regardless
 of which wait function it uses (wait, waitpid, or waitid), the
 parent still fails to determine the child's exit status.  I.e.,
 each of the wait functions does wait, but then returns -1/ECHILD
 rather than the child's PID.

That's as expected, according to POSIX
(http://www.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
section SIG_IGN).

 Which makes me think gnulib's wait-process.c will have to be changed
 to deal with this.

wait-process.c is meant to react depending on the exit code of the child
process. So the change I could apply is to add a comment in the specification:
This function assumes that the signal handler for SIGCHLD is not set to
SIG_IGN.

Do you have something else in mind?

Bruno


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Jim Meyering 
Bruno Haible wrote:
 Jim Meyering wrote:
 [reviving an old thread...]

 This was a reply to
 http://lists.gnu.org/archive/html/bug-coreutils/2008-10/msg00216.html,
 for those who - like me - lost the context after 4 months.

 I confirmed that running with SIGCHLD ignored, and regardless
 of which wait function it uses (wait, waitpid, or waitid), the
 parent still fails to determine the child's exit status.  I.e.,
 each of the wait functions does wait, but then returns -1/ECHILD
 rather than the child's PID.

 That's as expected, according to POSIX
 (http://www.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
 section SIG_IGN).

 Which makes me think gnulib's wait-process.c will have to be changed
 to deal with this.

 wait-process.c is meant to react depending on the exit code of the child
 process. So the change I could apply is to add a comment in the specification:
 This function assumes that the signal handler for SIGCHLD is not set to
 SIG_IGN.

Thanks.  That's probably all that it can do.
Though isn't it better to say it assumes the handler *is* set to SIG_DFL?

 Do you have something else in mind?

Hadn't investigated.
Obviously the set-sig-handler-to-default advice could only be
taken by an application, not by the library itself.

Though, at the other extreme, wait-process.c could print
a diagnostic about the potential portability problem when it
sees the SIGCHLD handler is not SIG_DFL.  Or return a value
indicating the problem -- esp. if it already knows it's running
on a system with the losing semantics.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Bruno Haible 
Jim Meyering wrote:
  the change I could apply is to add a comment in the specification:
  This function assumes that the signal handler for SIGCHLD is not set to
  SIG_IGN.
 
 Thanks.  That's probably all that it can do.

Done, see below.

 Though isn't it better to say it assumes the handler *is* set to SIG_DFL?

Why? When the signal handler is set to a function, wait_subprocess should
work, according to the description of waitpid() in POSIX.

 wait-process.c could print
 a diagnostic about the potential portability problem when it
 sees the SIGCHLD handler is not SIG_DFL.  Or return a value
 indicating the problem -- esp. if it already knows it's running
 on a system with the losing semantics.

Currently, when the problem occurs, it will print a diagnostic:
  subprogram subprocess: No child processes
This should be enough, I think.



2009-02-28  Bruno Haible  br...@clisp.org

* lib/wait-process.h (wait_subprocess): Clarify restriction regarding
SIGCHLD.
Reported by Jim Meyering.

--- lib/wait-process.h.orig 2009-02-28 17:40:00.0 +0100
+++ lib/wait-process.h  2009-02-28 17:39:40.0 +0100
@@ -1,5 +1,5 @@
 /* Waiting for a subprocess to finish.
-   Copyright (C) 2001-2003, 2006, 2008 Free Software Foundation, Inc.
+   Copyright (C) 2001-2003, 2006, 2008-2009 Free Software Foundation, Inc.
Written by Bruno Haible hai...@clisp.cons.org, 2001.
 
This program is free software: you can redistribute it and/or modify
@@ -50,7 +50,9 @@
  with an error status.
- If termsigp is not NULL, *termsig will be set to the signal that
  terminated the subprocess (if supported by the platform: not on native
- Windows platforms), otherwise 0.  */
+ Windows platforms), otherwise 0.
+   Prerequisites: The signal handler for SIGCHLD should not be set to SIG_IGN,
+   otherwise this function will not work.  */
 extern int wait_subprocess (pid_t child, const char *progname,
bool ignore_sigpipe, bool null_stderr,
bool slave_process, bool exit_on_error,


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Bruce Korb 
Hey, I remember this!  There was a long discussion about
SIGCHLD vs. SIGCLD / SysVR4 vs. BSD on the Austin reflector.
implementations are now allowed to reset the handler to
SIG_DFL on exec(2), so it is may not work not will not work.
Anyway, the final answer is that all programs that want
their zombies reaped should set the handler to a no-op
function at startup time.  Never use SIG_IGN for SIGCHLD,
period.  At all.  SIG_IGN will always be allowed to propagate
to exec-ed processes yielding bad results.  :(

Bruno Haible wrote:
 Jim Meyering wrote:
 the change I could apply is to add a comment in the specification:
 This function assumes that the signal handler for SIGCHLD is not set to
 SIG_IGN.

 @@ -50,7 +50,9 @@
   with an error status.
 - If termsigp is not NULL, *termsig will be set to the signal that
   terminated the subprocess (if supported by the platform: not on native
 - Windows platforms), otherwise 0.  */
 + Windows platforms), otherwise 0.
 +   Prerequisites: The signal handler for SIGCHLD should not be set to 
 SIG_IGN,
 +   otherwise this function will not work.  */
  extern int wait_subprocess (pid_t child, const char *progname,
   bool ignore_sigpipe, bool null_stderr,
   bool slave_process, bool exit_on_error,


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2009-02-28 Thread Bruno Haible 
Bruce Korb wrote:
 implementations are now allowed to reset the handler to
 SIG_DFL on exec(2)

Ah, an improvement indeed.

 so it is may not work not will not work.

No, it's still will not work because the text is talking about the
setting of SIGCHLD in the process that it calling wait_subprocess.
The setting in the child process has no effect here, and the setting
in the parent process is not directly relevant since it may have been
corrected in the current process.

Bruno


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-18 Thread Bruno Haible 
Eric Blake wrote:
  3. switch to the gnulib execute module, which makes this code portable to
  mingw (the current fork/exec sequence excludes mingw, which lacks fork),

Jim Meyering wrote:
 #3 sounds like it's worth a try, at least.

Note that we have now posix_spawn() portable to all Unix platforms, and I've
started looking at a possible implementation of posix_spawn() for mingw. 80%
of the building bricks are already present. The 'execute' module will then
be based on posix_spawn.

So, there is a chance that you get posix_spawn() supported for all platforms
within a month or two.  You will need less code refactoring if you want
to use posix_spawn than if you want to use the 'execute' module.

Bruno



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-18 Thread Bruno Haible 
Hi Eric,

 When used in a library, the gnulib fatal-signal module must assume that
 the rest of the program can use SIGPOLL and SIGALRM for its own purposes.
 But when used in a stand-alone app, such as csplit, where it is known in
 advance that these signals are not used, it would be nice to include them
 in the set of fatal signals.

Note that this makes the program harder to maintain. You add a call to sleep()
for debugging, and you suddenly see strange things... (There may be interactions
between sleep() and SIGALRM, see
http://www.gnu.org/software/libtool/manual/libc/Sleeping.html.)

Personally, I wouldn't change programs to react on SIGPOLL or SIGALRM that
are sent from outside the process. SIGPOLL and SIGALRM are not meant to be
used this way. The user who wants to kill a running process has enough
choice among SIGINT, SIGHUP, SIGTERM, SIGQUIT. Your proposed change would
negatively affect the _proper_ uses of SIGPOLL and SIGALRM in order to
improve the _improper_ uses of these signals.

 In other words, right now, csplit is _not_
 using fatal-signals, because it wants to react to additional signals
 beyond the default provided by the gnulib module.  Is there a way we could
 extend the API of fatal-signals to allow an application to request that
 SIGPOLL and SIGALRM be added to the set of signals to react to?

Yes, feel free to add such API to the 'fatal-signal' module. I propose a
function

  extern void init_fatal_signals (sigset_t signals_to_include,
  sigset_t signals_to_exclude);

that would change the set of fatal signals to

  (fatal_signals | signals_to_include)  ~signals_to_exclude

Bruno



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-16 Thread Jim Meyering 
Eric Blake [EMAIL PROTECTED] wrote:
...
 Subject: [PATCH] csplit: prefer sigaction over signal

 * bootstrap.conf (gnulib_modules): Import sigaction.
 * src/csplit.c (sigprocmask, siginterrupt) [SA_NOCLDSTOP]: Delete
 workarounds.
 (interrupt_handler, main): Drop use of signal.  Rely on sigaction
 to block fatal signal during cleanup, and to restore it to default
 in case of nested signals.

Impeccable!
Thank you.  Please push it.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-16 Thread Eric Blake 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

[adding bug-gnulib]

According to Jim Meyering on 10/16/2008 1:30 AM:
 Eric Blake [EMAIL PROTECTED] wrote:
 ...
 Subject: [PATCH] csplit: prefer sigaction over signal

 * bootstrap.conf (gnulib_modules): Import sigaction.
 * src/csplit.c (sigprocmask, siginterrupt) [SA_NOCLDSTOP]: Delete
 workarounds.
 (interrupt_handler, main): Drop use of signal.  Rely on sigaction
 to block fatal signal during cleanup, and to restore it to default
 in case of nested signals.

 Impeccable!
 Thank you.  Please push it.

Done.  Meanwhile, back to my original question:

According to Eric Blake on 10/13/2008 10:18 PM:
 Round 1: csplit.  The use of signal here mirrors what Bruno's fatal-signal
 module in gnulib provides, although the set of signals masked is
 different.  Which would you prefer, a patch that swaps over to
 fatal-signal usage (leaving at least SIGPOLL and SIGALRM unprotected), or
 this patch?  I'm kind of leaning towards simplifying this file by using
 fatal-signal, but had already written this, and wanted some feedback.


When used in a library, the gnulib fatal-signal module must assume that
the rest of the program can use SIGPOLL and SIGALRM for its own purposes.
 But when used in a stand-alone app, such as csplit, where it is known in
advance that these signals are not used, it would be nice to include them
in the set of fatal signals.  In other words, right now, csplit is _not_
using fatal-signals, because it wants to react to additional signals
beyond the default provided by the gnulib module.  Is there a way we could
extend the API of fatal-signals to allow an application to request that
SIGPOLL and SIGALRM be added to the set of signals to react to?

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkj3Mh4ACgkQ84KuGfSFAYD5FgCggtmDZhyYJM9vggcVQJuJuDZ+
aE8An0toPdDJdzN15j1G5zHmuMbADD+T
=ZQSs
-END PGP SIGNATURE-


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-16 Thread Eric Blake 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

According to Eric Blake on 10/10/2008 7:15 AM:
 I've wanted to get rid of signal uses for ages.
 Are you interested in doing it?
 
 Yes, I'll tackle this.

Round 2 begins with a question.  src/install.c has the following use of
signal, needed on the code path that invokes strip(1) as a child process:

#ifdef SIGCHLD
  /* System V fork+wait does not work if SIGCHLD is ignored.  */
  signal (SIGCHLD, SIG_DFL);
#endif

I just checked POSIX, and this is entirely true - the exec*() family is
allowed to inherit an ignored SIGCHLD, in deference to older systems like
SysV; and wait()/waitpid() are allowed to fail with ECHILD if SIGCHLD is
ignored.  But most systems these days explicitly set SIGCHLD to SIG_DFL on
fork; are there still any modern systems where the explicit signal change
is needed?

I see several options for this code in order to remove the use of signal:
1. delete the signal altogether, on the assumption that no modern system
inherits an ignored SIGCHLD
2. switch to sigaction to force SIGCHLD to SIG_DFL
3. switch to the gnulib execute module, which makes this code portable to
mingw (the current fork/exec sequence excludes mingw, which lacks fork),
and in the process isolates the portability problem of ignored SIGCHLD to
gnulib and makes it so that install.c need not mess with signal.h at all

I'm kind of leaning towards option 3, but want some agreement before
proceeding, as it is certainly more invasive to the current code, even
though it adds more portability as a result.

But in looking at the gnulib code for execute.c, I don't see any mention
of this SIGCHLD anomaly, where wait/waitpid fail if SIGCHLD is ignored.
On the other hand, gnulib's execute module uses waitid rather than
waitpid; I guess that this choice of API is immune to the SysV behavior?

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkj3PJQACgkQ84KuGfSFAYDb5ACfT34DqsCuMcdXfPnkuZ3N0wkf
EgkAmwYW8OMR53sJAdTggjOPvZ1BEfhY
=u7oV
-END PGP SIGNATURE-


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-16 Thread Jim Meyering 
Eric Blake [EMAIL PROTECTED] wrote:

 According to Eric Blake on 10/10/2008 7:15 AM:
 I've wanted to get rid of signal uses for ages.
 Are you interested in doing it?

 Yes, I'll tackle this.

 Round 2 begins with a question.  src/install.c has the following use of
 signal, needed on the code path that invokes strip(1) as a child process:

 #ifdef SIGCHLD
 /* System V fork+wait does not work if SIGCHLD is ignored.  */
 signal (SIGCHLD, SIG_DFL);
 #endif

 I just checked POSIX, and this is entirely true - the exec*() family is
 allowed to inherit an ignored SIGCHLD, in deference to older systems like
 SysV; and wait()/waitpid() are allowed to fail with ECHILD if SIGCHLD is
 ignored.  But most systems these days explicitly set SIGCHLD to SIG_DFL on
 fork; are there still any modern systems where the explicit signal change
 is needed?

 I see several options for this code in order to remove the use of signal:
 1. delete the signal altogether, on the assumption that no modern system
 inherits an ignored SIGCHLD
 2. switch to sigaction to force SIGCHLD to SIG_DFL
 3. switch to the gnulib execute module, which makes this code portable to
 mingw (the current fork/exec sequence excludes mingw, which lacks fork),
 and in the process isolates the portability problem of ignored SIGCHLD to
 gnulib and makes it so that install.c need not mess with signal.h at all

 I'm kind of leaning towards option 3, but want some agreement before
 proceeding, as it is certainly more invasive to the current code, even
 though it adds more portability as a result.

Invasive for a good cause ;-)
#3 sounds like it's worth a try, at least.

 But in looking at the gnulib code for execute.c, I don't see any mention
 of this SIGCHLD anomaly, where wait/waitpid fail if SIGCHLD is ignored.
 On the other hand, gnulib's execute module uses waitid rather than
 waitpid; I guess that this choice of API is immune to the SysV behavior?

Coreutils has a test for the install bug that prompted adding
the signal call: tests/install/trap, so at worst, we should discover
quickly whether execute.c needs a similar work-around.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-16 Thread Paul Eggert 
Eric Blake [EMAIL PROTECTED] writes:

 I just checked POSIX, and this is entirely true - the exec*() family is
 allowed to inherit an ignored SIGCHLD, in deference to older systems like
 SysV; and wait()/waitpid() are allowed to fail with ECHILD if SIGCHLD is
 ignored.  But most systems these days explicitly set SIGCHLD to SIG_DFL on
 fork; are there still any modern systems where the explicit signal change
 is needed?

I'd be a bit leery of assuming this.  It was such a pervasive problem
back in the day.  I'd be a bit surprised if the problem were eliminated
from all modern systems in all compilation modes.

 But in looking at the gnulib code for execute.c, I don't see any mention
 of this SIGCHLD anomaly, where wait/waitpid fail if SIGCHLD is ignored.
 On the other hand, gnulib's execute module uses waitid rather than
 waitpid; I guess that this choice of API is immune to the SysV behavior?

Yes, I expect so.

The simplest thing is to leave install.c alone.  Porting to mingw is low
priority, surely.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-13 Thread Eric Blake 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

According to Eric Blake on 10/10/2008 7:15 AM:
 I've wanted to get rid of signal uses for ages.
 Are you interested in doing it?
 
 Yes, I'll tackle this.

Round 1: csplit.  The use of signal here mirrors what Bruno's fatal-signal
module in gnulib provides, although the set of signals masked is
different.  Which would you prefer, a patch that swaps over to
fatal-signal usage (leaving at least SIGPOLL and SIGALRM unprotected), or
this patch?  I'm kind of leaning towards simplifying this file by using
fatal-signal, but had already written this, and wanted some feedback.

I think the SA_NODEFER | SA_RESETHAND combo is correct here; the handler
is for one-shot cleanup of temporary files, so we want the handler to be
reset to SIG_DFL on nested signal.  POSIX requires that portable programs
request SA_NODEFER alongside SA_RESETHAND, but we also include the signal
in the block mask, so a nested signal won't fire until after the handler
completes.  I think we are safe assuming that upon return from a signal
handler, a signal that is no longer blocked will fire.

$ git pull git://repo.or.cz/coreutils/ericb.git signal

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkj0HagACgkQ84KuGfSFAYDlqgCgjxLJHUN0KhgXmMEaejyBCgxm
sBEAnRFK4bdSrNxKpTvtR6KBQkBbcsCY
=PBR6
-END PGP SIGNATURE-
From 8c6dca936672e76f67c09e669bca803e80d64ef7 Mon Sep 17 00:00:00 2001
From: Eric Blake [EMAIL PROTECTED]
Date: Mon, 13 Oct 2008 22:04:51 -0600
Subject: [PATCH] csplit: prefer sigaction over signal

* bootstrap.conf (gnulib_modules): Import sigaction.
* src/csplit.c (sigprocmask, siginterrupt) [SA_NOCLDSTOP]: Delete
workarounds.
(interrupt_handler, main): Drop use of signal.  Rely on sigaction
to block fatal signal during cleanup, and to restore it to default
in case of nested signals.

Signed-off-by: Eric Blake [EMAIL PROTECTED]
---
 bootstrap.conf |2 +-
 src/csplit.c   |   30 --
 2 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index b3eec48..6405955 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -84,7 +84,7 @@ gnulib_modules=
safe-read same
save-cwd savedir savewd
selinux-at
-   settime sig2str ssize_t stat-macros
+   settime sig2str sigaction ssize_t stat-macros
stat-time stdbool stdlib-safer stpcpy
stpncpy
strftime
diff --git a/src/csplit.c b/src/csplit.c
index 55489c3..b50a650 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -34,17 +34,6 @@
 #include stdio--.h
 #include xstrtol.h
 
-/* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
-   present.  */
-#ifndef SA_NOCLDSTOP
-# define SA_NOCLDSTOP 0
-# define sigprocmask(How, Set, Oset) /* empty */
-# define sigset_t int
-# if ! HAVE_SIGINTERRUPT
-#  define siginterrupt(sig, flag) /* empty */
-# endif
-#endif
-
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME csplit
 
@@ -238,12 +227,10 @@ xalloc_die (void)
 static void
 interrupt_handler (int sig)
 {
-  if (! SA_NOCLDSTOP)
-signal (sig, SIG_IGN);
-
   delete_all_files (true);
-
-  signal (sig, SIG_DFL);
+  /* The signal has been reset to SIG_DFL, but blocked during this
+ handler.  Force the default action of this signal once the
+ handler returns and the block is removed.  */
   raise (sig);
 }
 
@@ -1421,7 +1408,6 @@ main (int argc, char **argv)
   };
 enum { nsigs = sizeof sig / sizeof sig[0] };
 
-#if SA_NOCLDSTOP
 struct sigaction act;
 
 sigemptyset (caught_signals);
@@ -1434,19 +1420,11 @@ main (int argc, char **argv)
 
 act.sa_handler = interrupt_handler;
 act.sa_mask = caught_signals;
-act.sa_flags = 0;
+act.sa_flags = SA_NODEFER | SA_RESETHAND;
 
 for (i = 0; i  nsigs; i++)
   if (sigismember (caught_signals, sig[i]))
sigaction (sig[i], act, NULL);
-#else
-for (i = 0; i  nsigs; i++)
-  if (signal (sig[i], SIG_IGN) != SIG_IGN)
-   {
- signal (sig[i], interrupt_handler);
- siginterrupt (sig[i], 1);
-   }
-#endif
   }
 
   split_file ();
-- 
1.6.0.2

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Jim Meyering 
Giuseppe Scrivano [EMAIL PROTECTED] wrote:
 do you have suggestions on this patch?  It replaces any `signal' with
 `sigaction'.

 Regards,
 Giuseppe Scrivano

 Jim Meyering [EMAIL PROTECTED] writes:

 Good idea.
 I've wanted to get rid of signal uses for ages.
 Are you interested in doing it?

Giuseppe,

Perhaps you misunderstood?
Eric proposed that idea and I addressed the reply above to him.
He then replied that he would indeed like to implement it.

Since you have spent time working on it, in spite of that,
we are now in a bind.

  - you don't have a copyright assignment on file for coreutils,
and those take at least 2-3 weeks to process.  So even if we
were to use your work, we'd have to wait.

  - it is considered poor form to jump in and write/submit a patch
like that, when someone else is obviously planning to do the
same thing.

I suggest that Eric go ahead and do what he offered to,
and post that.  Then, perhaps it'd be interesting to compare the
two patches, but I expect to review and use his.

If you'd like to contribute, please coordinate, especially if
it's a task that's being actively discussed.

If you'd like another idea, here's a task I'd like to see completed,
but for which no one has volunteered:

[excerpt from
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/14417/focus=14546 ]

[dd already has your b =512, ... notation, but 8 other programs
 with the same strings do not]

I've applied this, but omitted the above part, because it adds =
marks to just one of the several files that would need it in order to
keep those duplicated messages in sync.  However, I would welcome a patch
that unifies and (if possible) then factors out some of the duplication
displayed by this command:

grep -E 'MB? =?10..\*10' src/*.c

Ideally, each message string would appear in just one place,
and all clients would use some symbol from e.g., system.h.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Giuseppe Scrivano 
Hello,

Jim Meyering [EMAIL PROTECTED] writes:

 Giuseppe,

 Perhaps you misunderstood?
 Eric proposed that idea and I addressed the reply above to him.
 He then replied that he would indeed like to implement it.

Sorry, I thought it was directed to both.

 Since you have spent time working on it, in spite of that,
 we are now in a bind.

If it could be boring task for somebody already well introduced in
coreutils, it was a good occasion for me to familiarize with this
project.  In case the patch will not be accepted I don't have any
regrets for the time I spent on it.

   - you don't have a copyright assignment on file for coreutils,
 and those take at least 2-3 weeks to process.  So even if we
 were to use your work, we'd have to wait.

I know, last time it took more than one month.

   - it is considered poor form to jump in and write/submit a patch
 like that, when someone else is obviously planning to do the
 same thing.

I got his answer after I submitted the patch.  In the other case I think
I was showing my patch just to him not on the ML.
While the patch length is not trivial, the changes introduced by this
patch are very trivial.  It could be done easily with
s/signal/my_signal_wrapper/ but IMHO `signal' calls in coreutils are not
so many to require a wrapper around `sigaction'.

 If you'd like to contribute, please coordinate, especially if
 it's a task that's being actively discussed.

I will look more in detail at this task as soon as possible and inform
you if I will work on it.

Regards,
Giuseppe Scrivano


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Jim Meyering 
Pádraig Brady [EMAIL PROTECTED] wrote:
 Eric Blake wrote:
 On the other hand, POSIX is explicit that mixing signal and sigaction is
 not portable.  For that matter, now that gnulib provides a guaranteed
 sigaction, why don't we just change all of coreutils to use it?  Affected
 are: csplit, dd, install, ls, nohup, sort, tee, and timeout.

 Good idea.

 Perhaps we should just define our own signal()
 or bsd_signal() that calls sigaction() appropriately.
 I find signal() a much simpler interface than sigaction().

Hi Pádraig,

So do I.
I suppose it's worth considering, but signal is old and
unportable enough that I don't want to use that name.
Even bsd_signal carries baggage that we'd best avoid.

However, we have to weigh the cost of using a nonstandard
wrapper (readers would not recognize it right away) against
using POSIX's sigaction.

For now, let's look at the result after Eric's proposed signal-removing
clean-up/modernization is done.  Removing all of those in-function #ifdefs
is already a big improvement.  Maybe that'll be enough.  If not, you can
show us how much cleaner it can be using a simpler interface ;-)


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Andreas Schwab 
Giuseppe Scrivano [EMAIL PROTECTED] writes:

 +  memset (sa, 0, sizeof sa);
 +  sigemptyset (sa.sa_mask);

I don't think you need the memset.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Giuseppe Scrivano 
Andreas Schwab [EMAIL PROTECTED] writes:
 +  memset (sa, 0, sizeof sa);
 +  sigemptyset (sa.sa_mask);

 I don't think you need the memset.


and how reset the struct without a memset or using sa = {0,} as
Pádraig suggested?
Do you advise me to reset manually only members that really must be 0? 

Thanks,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-11 Thread Andreas Schwab 
Giuseppe Scrivano [EMAIL PROTECTED] writes:

 Andreas Schwab [EMAIL PROTECTED] writes:
 +  memset (sa, 0, sizeof sa);
 +  sigemptyset (sa.sa_mask);

 I don't think you need the memset.


 and how reset the struct without a memset or using sa = {0,} as
 Pádraig suggested?

You only need to additionally initialize sa_flags, the rest is already
initialized and doesn't need to be set twice.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady 
Giuseppe Scrivano wrote:
 Hi Pádraig,
 
 I think that the only problem can raise when the sent signal is received
 by the monitor process after the handler is reinstalled.  In that case
 the signal will be dispatched again to the process group.  This can
 repeat again and again until it is finally ignored by the monitor process.
 
 On http://www.opengroup.org/onlinepubs/009695399/functions/kill.html I
 can see in the Rationale section:
  There was initially strong sentiment to specify that, if pid
   specifies that a signal be sent to the calling process and that
   signal is not blocked, that signal would be delivered before kill()
   returns. This would permit a process to call kill() and be guaranteed
   that the call never return. .. Such modifications have the effect
   of satisfying the stronger requirement, at least when sigaction() is
   used, but not necessarily when signal() is used.
 
 So I guess the case I described before shouldn't happen.

Well found! From there:

Implementors are encouraged to meet the stronger requirement
[to deliver signal to calling process before kill() returns] whenever possible

So it's probably fine, and seems so on Linux from my testing,
but I don't think we should change it as I can't see any advantage.
I'm worried about (older) platforms that don't implement this _recommendation_

What I will do is remove the FIXME comment and replace it with your comments.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Eric Blake 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

According to Jim Meyering on 10/10/2008 6:44 AM:
 On the other hand, POSIX is explicit that mixing signal and sigaction is
 not portable.  For that matter, now that gnulib provides a guaranteed
 sigaction, why don't we just change all of coreutils to use it?  Affected
 are: csplit, dd, install, ls, nohup, sort, tee, and timeout.
 
 Good idea.
 I've wanted to get rid of signal uses for ages.
 Are you interested in doing it?

Yes, I'll tackle this.

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjvVYsACgkQ84KuGfSFAYBILgCgzF9lx87ZOMgbmf0j/fV24jKy
XusAn12Erhf7d/13Xo+5zLmUABqTuyM9
=K2CH
-END PGP SIGNATURE-


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady 
Eric Blake wrote:
 On the other hand, POSIX is explicit that mixing signal and sigaction is
 not portable.  For that matter, now that gnulib provides a guaranteed
 sigaction, why don't we just change all of coreutils to use it?  Affected
 are: csplit, dd, install, ls, nohup, sort, tee, and timeout.

Good idea.

Perhaps we should just define our own signal()
or bsd_signal() that calls sigaction() appropriately.
I find signal() a much simpler interface than sigaction().




___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady 
Giuseppe Scrivano wrote:
 Hello,
 
 do you have suggestions on this patch?  It replaces any `signal' with
 `sigaction'.

[snip]

 diff --git a/src/timeout.c b/src/timeout.c
 index 8b506f0..6ef7218 100644
 --- a/src/timeout.c
 +++ b/src/timeout.c
 @@ -278,8 +278,6 @@ main (int argc, char **argv)
/* Setup handlers before fork() so that we
   handle any signals caused by child, without races.  */
install_signal_handlers ();
 -  signal (SIGTTIN, SIG_IGN);/* don't sTop if background child needs tty. 
  */
 -  signal (SIGTTOU, SIG_IGN);/* don't sTop if background child needs tty. 
  */

Moving these after the fork() introduces a race
I think as per the comment.

 +  struct sigaction sa_ignore;
 +
 +  memset (sa_ignore, 0, sizeof sa_ignore);

Note since C90 this is equivalent to memset(0):
struct sigaction sa_ignore = {0,};

 +  sigemptyset (sa_ignore.sa_mask);
 +  sa_ignore.sa_handler = SIG_IGN;
 +
 +  sigaction (SIGTTIN, sa_ignore, NULL); /* don't sTop if background 
 child needs tty.  */
 +  sigaction (SIGTTOU, sa_ignore, NULL);/* don't sTop if background 
 child needs tty.  */

As I mentioned previously it might be better
for us to define our own function that calls sigaction,
but has the same interface as signal() as this is
a much cleaner interface and usually all that's required.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Eric Blake 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

According to Giuseppe Scrivano on 10/10/2008 2:34 PM:
 Hello,
 
 do you have suggestions on this patch?  It replaces any `signal' with
 `sigaction'.

The size of this patch is not trivial; are you willing to assign copyright
to FSF?  If so, I can get you started off-list.  Meanwhile, I don't want
to taint my chances of implementing this (I already have assignment), so I
will not offer any review.

- --
Don't work too hard, make some time for fun as well!

Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjwBTwACgkQ84KuGfSFAYCDrgCfdLZoUpFI1D1SgkpB+gCYRKL8
IWAAoIu/z2sWSeDwkL6qMuxJ+2uMs88w
=7gKH
-END PGP SIGNATURE-


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-09 Thread Pádraig Brady 
Pádraig Brady wrote:
 Giuseppe Scrivano wrote:
 Hello,

 what do you think about the following way to remove the sigs_to_ignore
 hack in the timeout.c file?
 It ignores temporarily the signal inside the `send_sig' function instead
 of using the `sigs_to_ignore' array.
 
 I'll test this tonight, as this stuff is tricky.

I tested it and couldn't break it,
though I'm worried about this change.

The code it replaces is very simple and explicit.
Whereas I'm worried about the implicit behaviour
of the new code. Specifically I'm worried about
races, where the monitor process may not receive
(and ignore) the signal before the signal handler
is reinstated. I.E. are we always guaranteed that
kill() is synchronous and will not return until
all signals have been delivered to all members of group?

thanks,
Pádraig,


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-07 Thread Pádraig Brady 
Giuseppe Scrivano wrote:
 Hello,
 
 what do you think about the following way to remove the sigs_to_ignore
 hack in the timeout.c file?
 It ignores temporarily the signal inside the `send_sig' function instead
 of using the `sigs_to_ignore' array.

I'll test this tonight, as this stuff is tricky.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils