Re: [PATCH v6 16/28] fsmonitor--daemon: stub in health thread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jeff,

On Tue, 17 May 2022, Jeff Hostetler wrote:

> On 5/12/22 11:05 AM, Johannes Schindelin wrote:
> >
> > On Fri, 22 Apr 2022, Jeff Hostetler via GitGitGadget wrote:
> >
> > > From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> > >
> > > Create another thread to watch over the daemon process and
> > > automatically shut it down if necessary.
> > >
> [...]
>
> > > The platform-specific code for Windows sets up enough of the
> > > WaitForMultipleObjects() machinery to watch for system and/or custom
> > > events.  Currently, the set of wait handles only includes our custom
> > > shutdown event (sent from our other theads).  Later commits in this
> > > series will extend the set of wait handles to monitor other
> > > conditions.
> [...]
>
> > > diff --git a/compat/fsmonitor/fsm-health-win32.c
> > > b/compat/fsmonitor/fsm-health-win32.c
> > > new file mode 100644
> > > index 00000000000..94b1d020f25
> > > --- /dev/null
> > > +++ b/compat/fsmonitor/fsm-health-win32.c
> > > @@ -0,0 +1,72 @@
> > > +#include "cache.h"
> > > +#include "config.h"
> > > +#include "fsmonitor.h"
> > > +#include "fsm-health.h"
> > > +#include "fsmonitor--daemon.h"
> > > +
> > > +struct fsm_health_data
> > > +{
> > > +	HANDLE hEventShutdown;
> > > +
> > > +	HANDLE hHandles[1]; /* the array does not own these handles */
> > > +#define HEALTH_SHUTDOWN 0
> >
> > How about defining `HANDLE hHandles[HEALTH_SHUTDOWN + 1]` to indicate that
> > the constant is used as an offset into `hHandles`?
> >
> > > +	int nr_handles; /* number of active event handles */
>
> I think I'd like to keep this one as is.  It matches the style that
> I used in `fsm-listen-win32.c` where I have 3 listener handles.
>
> Granted, it does look a little odd when there is only 1 handle in the
> array.  But the idea was to allow new handles to be added as we want
> to watch more things.
>
> It might be clearer (in both of them) to define the array in
> terms of an enum rather than a local list of #define's.
> But I'm not sure it matters.

Your explanation makes sense to me, I have no objections against keeping
the code as-is.

Thanks,
Dscho




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux