Re: [PATCH] Fix a signal handler

[Markus Elfring <Markus.Elfring@web.de>]

> Of are you asking me if I'd apply your patch if you send a polished update,
> and asking me to decide it before seeing the patch?

Would you like to pick this source code adjustment up?

Regards,
Markus

---
From e138904a08ceaf469fa2f4d0ec87b5891be14760 Mon Sep 17 00:00:00 2001
From: Markus Elfring <Markus.Elfring@web.de>
Date: Mon, 22 Feb 2010 11:53:35 +0100
Subject: [PATCH] Fix a signal handler

A global flag can only be set by a signal handler in a portable way
if it has got the data type "sig_atomic_t". The previously used assignment
of a function pointer in the function "early_output" was moved to another
variable in the function "setup_early_output".

The involved software design details were also mentioned on the mailing list.

Signed-off-by: Markus Elfring <Markus.Elfring@web.de>

---
 builtin-log.c |   10 ++--------
 revision.c    |   14 ++++++--------
 revision.h    |    3 ++-
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index e0d5caa..beccf7f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -170,20 +170,14 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
 
 static void early_output(int signal)
 {
-	show_early_output = log_show_early;
+	show_early_output = 1;
 }
 
 static void setup_early_output(struct rev_info *rev)
 {
 	struct sigaction sa;
 
-	/*
-	 * Set up the signal handler, minimally intrusively:
-	 * we only set a single volatile integer word (not
-	 * using sigatomic_t - trying to avoid unnecessary
-	 * system dependencies and headers), and using
-	 * SA_RESTART.
-	 */
+	early_output_function = &log_show_early;
 	memset(&sa, 0, sizeof(sa));
 	sa.sa_handler = early_output;
 	sigemptyset(&sa.sa_mask);
diff --git a/revision.c b/revision.c
index 3ba6d99..62402fb 100644
--- a/revision.c
+++ b/revision.c
@@ -13,7 +13,8 @@
 #include "decorate.h"
 #include "log-tree.h"
 
-volatile show_early_output_fn_t show_early_output;
+sig_atomic_t show_early_output = 0;
+show_early_output_fn_t early_output_function = NULL;
 
 char *path_name(const struct name_path *path, const char *name)
 {
@@ -654,7 +655,6 @@ static int limit_list(struct rev_info *revs)
 		struct commit_list *entry = list;
 		struct commit *commit = list->item;
 		struct object *obj = &commit->object;
-		show_early_output_fn_t show;
 
 		list = list->next;
 		free(entry);
@@ -680,12 +680,10 @@ static int limit_list(struct rev_info *revs)
 		date = commit->date;
 		p = &commit_list_insert(commit, p)->next;
 
-		show = show_early_output;
-		if (!show)
-			continue;
-
-		show(revs, newlist);
-		show_early_output = NULL;
+		if (show_early_output) {
+			(*early_output_function)(revs, newlist);
+			show_early_output = 0;
+		}
 	}
 	if (revs->cherry_pick)
 		cherry_pick_list(newlist, revs);
diff --git a/revision.h b/revision.h
index a14deef..93a8ffc 100644
--- a/revision.h
+++ b/revision.h
@@ -135,7 +135,8 @@ struct rev_info {
 
 /* revision.c */
 typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);
-extern volatile show_early_output_fn_t show_early_output;
+extern show_early_output_fn_t early_output_function;
+extern sig_atomic_t show_early_output;
 
 extern void init_revisions(struct rev_info *revs, const char *prefix);
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
-- 
1.7.0