[ovs-dev] [PATCH 2/4] fatal-signal: Clean up code by using shash.
blp at nicira.com
Mon Sep 21 23:58:14 UTC 2009
Justin Pettit <jpettit at nicira.com> writes:
> On Sep 21, 2009, at 4:41 PM, Ben Pfaff wrote:
>>> I don't suppose you care, but "files" is never destroyed. It wasn't
>>> done when it was an array, either so it is consistent. It only
>>> matters when do_unlink_files() is called and the program is probably
>>> on its way out anyways...
>> That is intentional and necessary. do_unlink_files() gets called
>> from an asynchronous signal handler. If the signal arrives
>> during a call to malloc() or free(), etc., then freeing the data
>> structure could cause a segmentation fault.
> Do you think that's worth documenting?
You mean you want me to comment something that is important yet
extraordinarily unobvious to the casual reader? Sheesh, you are
Author: Ben Pfaff <blp at nicira.com>
Date: Mon Sep 21 16:56:37 2009 -0700
fatal-signal: Add clarifying comments.
Suggested by Justin Pettit.
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index d8862bb..8180521 100644
@@ -60,7 +60,11 @@ static void call_hooks(int sig_nr);
/* Registers 'hook' to be called when a process termination signal is raised.
* If 'run_at_exit' is true, 'hook' is also called during normal process
- * termination, e.g. when exit() is called or when main() returns. */
+ * termination, e.g. when exit() is called or when main() returns.
+ * 'func' will be invoked from an asynchronous signal handler, so it must be
+ * written appropriately. For example, it must not call most C library
+ * functions, including malloc() or free(). */
fatal_signal_add_hook(void (*func)(void *aux), void *aux, bool run_at_exit)
@@ -228,6 +232,12 @@ unlink_files(void *aux UNUSED)
+/* This is a fatal_signal_add_hook() callback (via unlink_files()). It will be
+ * invoked from an asynchronous signal handler, so it cannot call most C
+ * library functions (unlink() is an explicit exception, see
+ * http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html).
+ * That includes free(), so it doesn't try to free the 'files' data
+ * structure. */
More information about the dev