>
> /*===== Signal's list to be managed ===========================*/
> /*==============================================================*/
> /*
> * in this example, we want to handle SIGINT, SIGQUIT and SIGTERM
> */
> static int psig_list[] = {SIGINT, SIGQUIT, SIGTERM};
>
>
> /*===== Usefull macros =========================================*/
> /*==============================================================*/
>
> #define NELEMS_ARRAY(array) ( sizeof(array)/sizeof(array[0]) )
>
> #define DIE(text) \
> do { \
> perror ("Error description: "); \
I generally prefer to see 'strerror()' used with [fs]printf
rather than perror(). perror cannot be used with most of the
pthread-related interfaces because they don't set errno.
(NOTE TO SELF: Submit ardvaark on perror() to add application usage
regarding use of perror() with threaded applications (i.e. don't do it!)).
The errno value should be a parameter to DIE and strerror()
should be used to format the string for it.
> fprintf (stderr, \
> "|___ At %s():%d, error caused by %s\n", \
> __func__, \
__func__ is non standard C.
> __LINE__, \
> text \
> ); \
> exit (EXIT_FAILURE); \
> } while(0)
>
[elided]
>
> int main(int argc,
> char **argv
> )
> {
> pthread_t sig_thr_id; /* signal handler thread ID */
> sigset_t sig_to_block; /* signals to block */
> int rc; /* return code */
>
> /*------------------------------------------------------------*/
> /* Install the mechanism to deal with the "process-oriented" */
> /* signals listed in psig_list[]. */
> /*------------------------------------------------------------*/
> /*
> * Set the initial thread's signal mask to block
> * the signals listed in psig_list
> */
> create_sigmask (&sig_to_block,
> psig_list,
> NELEMS_ARRAY(psig_list)
> );
> rc = pthread_sigmask (SIG_BLOCK,
> &sig_to_block,
> (sigset_t*) NULL
> );
> if (rc < 0)
The return value from pthread_sigmask is defined to be zero for
success, or a positive errno value for error. This check is in
error.
More generally, even the traditional functions define '-1' as
the error return, not a negative number, so "if (rc < 0)" is almost
never correct. if (rc == -1) should be used instead to detect errors.
(ran in to this years ago with lseek() against /proc/pid/mem on SVR4 where
a negative return value is legal when addresses have the MSB set).
(the best check for lseek, with SEEK_SET, is to check that the return
value matches the supplied offset value).
I'd prefer to see compound statements used (e.g. {}) with conditional
statements (if, do, while) even if there is only one statement in the
block.
> DIE("pthread_sigmask()"); /* die on error */
errno is _not_ set by pthread_sigmask, so the perror() in DIE
is ineffective.
>
> /*
> * Start a dedicated thread to handle signalling
> */
> rc = pthread_create (&sig_thr_id,
> (pthread_attr_t*) NULL,
> (pthread_routine_t) signal_thread,
> (void*) NULL
> );
> if (rc < 0)
Again, the return code is positive on error, and errno is _not_ set,
so the perror() call in DIE will not work!.
> DIE("pthread_create()"); /* die on error */
> /*
> int rc; /* returned code */
>
> /*------------------------------------------------------------*/
> /* wait for one signal in psig_list[] to become pending */
> /*------------------------------------------------------------*/
> create_sigmask (&sig_to_wait4,
> psig_list,
> NELEMS_ARRAY(psig_list)
> );
> rc = sigwait (&sig_to_wait4,
> &sig_caught
> );
> if (rc < 0)
> DIE("sigwait()"); /* die on error */
Again, the return code from sigwait is mishandled. See above.
> int rc;
>
> /*------------------------------------------------------------*/
> /* Set sigmask to be the union of signal listed list[] */
> /*------------------------------------------------------------*/
> rc = sigemptyset (ptr_sigmask);
> for (i=0; i<n; i++)
> {
> rc = sigaddset (ptr_sigmask,
> list[i]
> );
> if (rc < 0)
The return value for sigaddset is defined to return "-1" on
error. This check should be "if (rc == -1) { }"
errno should be passed to DIE as a parameter.
> DIE("sigaddset()"); /* die on error */
scott
|