Subject: Re: [PATCH v2 11/20] sigchain: move doc to sigchain.h

From: Emily Shaffer <[email-masked]>
Date: Wed, 6 Nov 2019 14:03:24 -0800

On Wed, Nov 06, 2019 at 09:59:38AM +0000, Heba Waly via GitGitGadget wrote:
> From: Heba Waly <[email-masked]>
> 
> Move the documentation from Documentation/technical/api-sigchain.txt
> to sigchain.h as it's easier for the developers to find the usage
> information beside the code instead of looking for it in another doc file.
> 
> Also documentation/technical/api-sigchain.txt is removed because the
> information it has is now redundant and it'll be hard to keep it up to
> date and synchronized with the documentation in the header file.
> 
> Signed-off-by: Heba Waly <[email-masked]>
> ---
>  Documentation/technical/api-sigchain.txt | 41 -----------------------
>  sigchain.h                               | 42 ++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/technical/api-sigchain.txt
> 
> diff --git a/Documentation/technical/api-sigchain.txt b/Documentation/technical/api-sigchain.txt
> deleted file mode 100644
> index 9e1189ef01..0000000000
> --- a/Documentation/technical/api-sigchain.txt
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -sigchain API
> -============
> -
> -Code often wants to set a signal handler to clean up temporary files or
> -other work-in-progress when we die unexpectedly. For multiple pieces of
> -code to do this without conflicting, each piece of code must remember
> -the old value of the handler and restore it either when:
> -
> -  1. The work-in-progress is finished, and the handler is no longer
> -     necessary. The handler should revert to the original behavior
> -     (either another handler, SIG_DFL, or SIG_IGN).
> -
> -  2. The signal is received. We should then do our cleanup, then chain
> -     to the next handler (or die if it is SIG_DFL).
> -
> -Sigchain is a tiny library for keeping a stack of handlers. Your handler
> -and installation code should look something like:
> -
> -------------------------------------------
> -  void clean_foo_on_signal(int sig)
> -  {
> -	  clean_foo();
> -	  sigchain_pop(sig);
> -	  raise(sig);
> -  }
> -
> -  void other_func()
> -  {
> -	  sigchain_push_common(clean_foo_on_signal);
> -	  mess_up_foo();
> -	  clean_foo();
> -  }
> -------------------------------------------
> -
> -Handlers are given the typedef of sigchain_fun. This is the same type
> -that is given to signal() or sigaction(). It is perfectly reasonable to
> -push SIG_DFL or SIG_IGN onto the stack.
> -
> -You can sigchain_push and sigchain_pop individual signals. For
> -convenience, sigchain_push_common will push the handler onto the stack
> -for many common signals.
> diff --git a/sigchain.h b/sigchain.h
> index 138b20f54b..a990f18cf6 100644
> --- a/sigchain.h
> +++ b/sigchain.h
> @@ -1,12 +1,54 @@
>  #ifndef SIGCHAIN_H
>  #define SIGCHAIN_H
>  
> +/**
> + * Code often wants to set a signal handler to clean up temporary files or
> + * other work-in-progress when we die unexpectedly. For multiple pieces of
> + * code to do this without conflicting, each piece of code must remember
> + * the old value of the handler and restore it either when:
> + *
> + *   1. The work-in-progress is finished, and the handler is no longer
> + *      necessary. The handler should revert to the original behavior
> + *      (either another handler, SIG_DFL, or SIG_IGN).
> + *
> + *   2. The signal is received. We should then do our cleanup, then chain
> + *      to the next handler (or die if it is SIG_DFL).
> + *
> + * Sigchain is a tiny library for keeping a stack of handlers. Your handler
> + * and installation code should look something like:
> + *
> + * ------------------------------------------
> + *   void clean_foo_on_signal(int sig)
> + *   {
> + * 	  clean_foo();
> + * 	  sigchain_pop(sig);
> + * 	  raise(sig);
> + *   }
> + *
> + *   void other_func()
> + *   {
> + * 	  sigchain_push_common(clean_foo_on_signal);
> + * 	  mess_up_foo();
> + * 	  clean_foo();
> + *   }
> + * ------------------------------------------
> + *
> + */
> +
> +/**
> + * Handlers are given the typedef of sigchain_fun. This is the same type
> + * that is given to signal() or sigaction(). It is perfectly reasonable to
> + * push SIG_DFL or SIG_IGN onto the stack.
> + */
>  typedef void (*sigchain_fun)(int);
>  
> +/* You can sigchain_push and sigchain_pop individual signals. */
>  int sigchain_push(int sig, sigchain_fun f);
>  int sigchain_pop(int sig);
>  
> +/* push the handler onto the stack for many common signals. */
It was lacking in the original doc too, but I want to know which common
signals it pushes for. Is it too much work for you to peek in the
implementation and let us know?
>  void sigchain_push_common(sigchain_fun f);
> +
>  void sigchain_pop_common(void);
>  
>  #endif /* SIGCHAIN_H */
> -- 
> gitgitgadget
> 

Otherwise this one looks fine for me.

 - Emily