On Sat, Dec 17, 2016 at 5:46 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> Yeah, but we are planning to add a generic flag in WaitEvent structure
> which can be used for similar purpose. However, as per your
> suggestion, I have changed it only for Win32 port. Also, I kept the
> change in ModifyWaitEvent API as that seems to be okay considering
> 'reset' is a generic flag in WaitEvent structure.
Well, we don't really need the change in ModifyWaitEvent if we have
the change in WaitEventSetWaitBlock, right? I'd be inclined to ditch
the former and keep the latter. Also, this doesn't look right:
+ for (cur_event = set->events;
+ cur_event < (set->events + set->nevents)
+ && returned_events < nevents;
+ cur_event++)
+ {
+ if (cur_event->reset)
+ {
+ WaitEventAdjustWin32(set, cur_event);
+ cur_event->reset = false;
+ }
+ }
There's no need to include returned_events < nevents in the loop
condition here because returned_events isn't changing.
I think I'd also guard the reset flag with #ifdef WIN32. If it's not
properly supported elsewhere it's just a foot-gun, and there seems to
be no reason to write the code to properly support it elsewhere,
whatever that would mean.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company