On 19 Oct 2011, at 17:54, Florian Pflug wrote:
> On Oct19, 2011, at 17:47 , Greg Jaskiewicz wrote:
>> On 15 Oct 2011, at 11:31, Florian Pflug wrote:
>>>
>>> Ok, here's a first cut.
>>
>> So I looked at the patch, and first thing that pops out,
>> is lack of the volatile keyword before the ClientConnectionLostPending variable is defined. Is that done on purpose
?Is that on purpose ?
>
> That's on purpose. volatile is only necessary for variables which are either accessed from within signal handlers or
whichlive in shared memory. Neither is true for ClientConnectionLostPending, so non-volatile should be fine.
Ok, cool.
I'm aware of the reasons behind volatile, just noticed that some other flags used in similar way are marked as such. At
theend of the day, this is just a hint to the compiler anyway.
>
>> Otherwise the patch itself looks ok.
>> I haven't tested the code, just reviewed the patch itself. And it obviously needs testing, should be easy to follow
youroriginal problem description.
>
> Yeah, further testing is on my todo list. The interesting case is probably what happens if the connection is dropped
whilethere's already a cancellation request pending. And also the other way around - a cancellation request arriving
afterwe've already discovered that the connection is gone.
>
>> Btw, I just tried to do it through commitfest.postgresql.org , but before I get my head around on how to add myself
tothe reviewer list there - I thought I'll just send this response here.
>
> You just need to click "Edit Patch" and put your name into the Reviewer field. You do need a postgres community
accountto edit patches, but the signup process is quite quick and painless AFAIR.
Ok, clicking 'edit patch' sounds a bit big. Probably better would be, to just be able to click on some sort of "I'm in"
button/checkboxtype of thing.