[SSSD] [PATCHES] Sig handlers cleanups

Jakub Hrozek jhrozek at redhat.com
Thu Jan 8 10:35:40 UTC 2015


On Thu, Jan 08, 2015 at 11:18:23AM +0100, Lukas Slebodnik wrote:
> On (08/01/15 11:10), Pavel Reichl wrote:
> >
> >On 01/08/2015 10:24 AM, Jakub Hrozek wrote:
> >>On Mon, Jan 05, 2015 at 06:48:29PM +0100, Pavel Reichl wrote:
> >>>On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
> >>>>On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
> >>>>>On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
> >>>>>>On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
> >>>>>>>On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
> >>>>>>>>On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
> >>>>>>>>>On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
> >>>>>>>>>>On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
> >>>>>>>>>>>On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
> >>>>>>>>>>>>While checking if our custom signal handlers properly handle errno, I
> >>>>>>>>>>>>stumbled on a few cleanups, they are attached.
> >>>>>>>>>>>>
> >>>>>>>>>>>>turns out our few signal hanlders are errno safe, and tevent signal
> >>>>>>>>>>>>handling function is also fine.
> >>>>>>>>>>>>
> >>>>>>>>>>>>Simo.
> >>>>>>>>>>>>
> >>>>>>>>>>>>-- 
> >>>>>>>>>>>>Simo Sorce * Red Hat, Inc * New York
> >>>>>>>>>>>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001
> >>>>>>>>>>>>From: Simo Sorce <simo at redhat.com>
> >>>>>>>>>>>>Date: Sat, 9 Nov 2013 16:11:04 -0500
> >>>>>>>>>>>>Subject: [PATCH 2/3] Signals: Remove empty sig_hup
> >>>>>>>>>>>>
> >>>>>>>>>>>>SIGHUP handling is simply not implemented, so just block the signal instead of
> >>>>>>>>>>>it is, see te_server_hup() and the usage is documented in the sssd man
> >>>>>>>>>>>page.
> >>>>>>>>>>>
> >>>>>>>>>>>>using a fake handler.
> >>>>>>>>>>Ok does it mean I need to remove the part where I block the signal ?
> >>>>>>>>>no, but you have to unblock it in server_setup() and if it is harmless
> >>>>>>>>>to unblock a signal twice I would unblock it in monitor_process_init()
> >>>>>>>>>before calling tevent_add_signal() as well.
> >>>>>>>>>
> >>>>>>>>>>Do you have any other comment ?
> >>>>>>>>>no, but I still need to test them.
> >>>>>>>>>
> >>>>>>>>>>The sig_hup(int sig) function *was* useless ? Or is it there as a place
> >>>>>>>>>>holder until te_server_hup() takes over ?
> >>>>>>>>>no, I think the common patter here is to block everything in
> >>>>>>>>>setup_signals() and then unblock the signal when the related
> >>>>>>>>>tevent_add_signal() is called.
> >>>>>>>>Ok after looking more carefully at how we setup the tevent handlers for
> >>>>>>>>SIGHUP I just reverted the part of the patch that would block the
> >>>>>>>>signal, it makes no sense to block it just to revert the action a few
> >>>>>>>>microseconds later.
> >>>>>>>>
> >>>>>>>>New patch 0002 attached.
> >>>>>>>Thank you. With this patch SSSD receives and handles all documented
> >>>>>>>signals.
> >>>>>>>
> >>>>>>>ACK
> >>>>>>>
> >>>>>>>While testing I came across an oddness which we might want to fix. The
> >>>>>>>sssd_be processes can receive SIGUSR1 individually and go offline. But
> >>>>>>>the SIGUSR2 is not handled by sssd_be directly but only by the monitor
> >>>>>>>which then send a reset offline request to all backends. Shall sssd_be
> >>>>>>>handle SIGUSR2 as well?
> >>>>>>>
> >>>>>>>bye,
> >>>>>>>Sumit
> >>>>>>Which patches should be pushed? #1 and #3 from the original mail and #2
> >>>>>>from the follow up?
> >>>>>yes
> >>>>>
> >>>>>bye,
> >>>>>Sumit
> >>>>OK, pushed those three to master.
> >>>Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is
> >>>installed.
> >>>These problem seem to be solved when patch 'Signals: Remove unused
> >>>functions' is applied.
> >>>Can we push it to 1-11 branch?
> >>I don't have a problem with pushing a patch that removes unused static
> >>functions, although Samba really shouldn't export non-namespaced
> >>functions.
> >>
> >>Is that another manifestation of:
> >>     https://bugzilla.samba.org/show_bug.cgi?id=11033
> >>?
> >I think the root of the problem is same - samba provides in its public
> >headers (ndr.h includes samba_util.h) declarations of its functions (void
> >(*CatchChild(void))(int);  void (*CatchChildLeaveStatus(void))(int);)  which
> >conflicts with SSSD function's (void CatchChild(void); void
> >CatchChildLeaveStatus(void);)
> >
> Actually, our file src/util/signal.c was copied from samba code.
> @see header of that file. So we should have used prefixed version as well.

I don't think static functions need to be prefixed.

The real problem was someone included an internal header in a public
one, it seems.



More information about the sssd-devel mailing list