Message ID | 22d591176c7b3512ceabddf439a6d6ab682288d1.1466409587.git.mschiffer@universe-factory.net |
---|---|
State | Superseded |
Headers | show |
On 06/20/2016 09:59 AM, Matthias Schiffer wrote: > When a process quits in response to a signal it handles, it should to so > be re-sending the signal to itself. This especially important for SIGINT, > as is explained in [1]. > > uloop currently hides the reason for quitting uloop_run(). Fix this by > returning the signal that caused the loop to quit (or 0 when uloop_end() > was used), so a program using loop an comply with [1]. > > uloop_cancelled is renamed to uloop_status, so accidentially running a > process compiled with one uloop_cancelled definition against the other is > not possible. I've just noticed that this will also require a ubus change, as libubus is accessing uloop_cancelled directly. Would you prefer to keep uloop_cancelled with its old definition and add ubus_status as an additional variable to keep this compatible? An alternative would be to provide a helper uloop_is_cancelled() based on ubus_status and adjust libubus accordingly. Regards, Matthias > > [1] https://www.cons.org/cracauer/sigint.html > > Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
On 06/20/2016 09:59 AM, Matthias Schiffer wrote: > When a process quits in response to a signal it handles, it should to so > be re-sending the signal to itself. This especially important for SIGINT, > as is explained in [1]. > > uloop currently hides the reason for quitting uloop_run(). Fix this by > returning the signal that caused the loop to quit (or 0 when uloop_end() > was used), so a program using loop an comply with [1]. > > uloop_cancelled is renamed to uloop_status, so accidentially running a > process compiled with one uloop_cancelled definition against the other is > not possible. > > [1] https://www.cons.org/cracauer/sigint.html > > Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> Acked-by: Jo-Philipp Wich <jo@mein.io> > --- > uloop.c | 14 ++++++++------ > uloop.h | 6 +++--- > 2 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/uloop.c b/uloop.c > index cd3de85..fa3544d 100644 > --- a/uloop.c > +++ b/uloop.c > @@ -57,7 +57,7 @@ static struct list_head timeouts = LIST_HEAD_INIT(timeouts); > static struct list_head processes = LIST_HEAD_INIT(processes); > > static int poll_fd = -1; > -bool uloop_cancelled = false; > +int uloop_status = -1; > static bool do_sigchld = false; > > static struct uloop_fd_event cur_fds[ULOOP_MAX_EVENTS]; > @@ -330,7 +330,7 @@ static void uloop_handle_processes(void) > > static void uloop_handle_sigint(int signo) > { > - uloop_cancelled = true; > + uloop_status = signo; > } > > static void uloop_sigchld(int signo) > @@ -443,7 +443,7 @@ static void uloop_clear_processes(void) > uloop_process_delete(p); > } > > -void uloop_run(void) > +int uloop_run(void) > { > static int recursive_calls = 0; > struct timeval tv; > @@ -455,8 +455,8 @@ void uloop_run(void) > if (!recursive_calls++) > uloop_setup_signals(true); > > - uloop_cancelled = false; > - while(!uloop_cancelled) > + uloop_status = -1; > + while (uloop_status < 0) > { > uloop_gettime(&tv); > uloop_process_timeouts(&tv); > @@ -464,7 +464,7 @@ void uloop_run(void) > if (do_sigchld) > uloop_handle_processes(); > > - if (uloop_cancelled) > + if (uloop_status >= 0) > break; > > uloop_gettime(&tv); > @@ -473,6 +473,8 @@ void uloop_run(void) > > if (!--recursive_calls) > uloop_setup_signals(false); > + > + return uloop_status; > } > > void uloop_done(void) > diff --git a/uloop.h b/uloop.h > index 7564514..4d87d46 100644 > --- a/uloop.h > +++ b/uloop.h > @@ -83,7 +83,7 @@ struct uloop_process > pid_t pid; > }; > > -extern bool uloop_cancelled; > +extern int uloop_status; > extern bool uloop_handle_sigchld; > > int uloop_fd_add(struct uloop_fd *sock, unsigned int flags); > @@ -99,11 +99,11 @@ int uloop_process_delete(struct uloop_process *p); > > static inline void uloop_end(void) > { > - uloop_cancelled = true; > + uloop_status = 0; > } > > int uloop_init(void); > -void uloop_run(void); > +int uloop_run(void); > void uloop_done(void); > > #endif >
diff --git a/uloop.c b/uloop.c index cd3de85..fa3544d 100644 --- a/uloop.c +++ b/uloop.c @@ -57,7 +57,7 @@ static struct list_head timeouts = LIST_HEAD_INIT(timeouts); static struct list_head processes = LIST_HEAD_INIT(processes); static int poll_fd = -1; -bool uloop_cancelled = false; +int uloop_status = -1; static bool do_sigchld = false; static struct uloop_fd_event cur_fds[ULOOP_MAX_EVENTS]; @@ -330,7 +330,7 @@ static void uloop_handle_processes(void) static void uloop_handle_sigint(int signo) { - uloop_cancelled = true; + uloop_status = signo; } static void uloop_sigchld(int signo) @@ -443,7 +443,7 @@ static void uloop_clear_processes(void) uloop_process_delete(p); } -void uloop_run(void) +int uloop_run(void) { static int recursive_calls = 0; struct timeval tv; @@ -455,8 +455,8 @@ void uloop_run(void) if (!recursive_calls++) uloop_setup_signals(true); - uloop_cancelled = false; - while(!uloop_cancelled) + uloop_status = -1; + while (uloop_status < 0) { uloop_gettime(&tv); uloop_process_timeouts(&tv); @@ -464,7 +464,7 @@ void uloop_run(void) if (do_sigchld) uloop_handle_processes(); - if (uloop_cancelled) + if (uloop_status >= 0) break; uloop_gettime(&tv); @@ -473,6 +473,8 @@ void uloop_run(void) if (!--recursive_calls) uloop_setup_signals(false); + + return uloop_status; } void uloop_done(void) diff --git a/uloop.h b/uloop.h index 7564514..4d87d46 100644 --- a/uloop.h +++ b/uloop.h @@ -83,7 +83,7 @@ struct uloop_process pid_t pid; }; -extern bool uloop_cancelled; +extern int uloop_status; extern bool uloop_handle_sigchld; int uloop_fd_add(struct uloop_fd *sock, unsigned int flags); @@ -99,11 +99,11 @@ int uloop_process_delete(struct uloop_process *p); static inline void uloop_end(void) { - uloop_cancelled = true; + uloop_status = 0; } int uloop_init(void); -void uloop_run(void); +int uloop_run(void); void uloop_done(void); #endif
When a process quits in response to a signal it handles, it should to so be re-sending the signal to itself. This especially important for SIGINT, as is explained in [1]. uloop currently hides the reason for quitting uloop_run(). Fix this by returning the signal that caused the loop to quit (or 0 when uloop_end() was used), so a program using loop an comply with [1]. uloop_cancelled is renamed to uloop_status, so accidentially running a process compiled with one uloop_cancelled definition against the other is not possible. [1] https://www.cons.org/cracauer/sigint.html Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> --- uloop.c | 14 ++++++++------ uloop.h | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-)