On Tue, Dec 20, 2016 at 8:04 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> 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?
>
Yes.
> I'd be inclined to ditch
> the former and keep the latter.
>
Okay, I think you want to keep the change just for Win32 which seems
fair as we haven't noticed such problem on any other platform.
> 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.
>
Fixed.
> 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.
>
Okay.
Ashutosh Sharma has helped to test that pldebugger issue is fixed with
attached version.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers