Rainer,

On 3/11/15 12:38 PM, Rainer Jung wrote:
> Am 11.03.2015 um 15:45 schrieb Mark Thomas:
>> On 11/03/2015 14:44, ma...@apache.org wrote:
>>> Author: markt
>>> Date: Wed Mar 11 14:44:23 2015
>>> New Revision: 1665888
>>>
>>> URL: http://svn.apache.org/r1665888
>>> Log:
>>> Fix 57653. Crash when multiple events for same socket are returned
>>> via separate apr_pollfd_t structures
>>
>> Review appreciated from folks that understand C better than I do (i.e.
>> pretty much anybody). Hopefully it is clear from the comment what I am
>> trying to do here and why.
> 
> I guess it is more of a question of knowing the pollset and RING magic -
> which I don't really qualify for - but it looks good to me.

+1

> Even if for
> the two fd with the same socket their client_data parts might not be
> identical, they should point to the same (identical) socket structure,
> so NULLing "pe" in the socket during the first fd occurence should work
> as a decision point for the second occurence.

+1

Is it a problem to call apr_pollset_remove for that file descriptor if
it's not actually in the pollset? That is, are remove operations
idempotent? Would it be considered an error (at a higher level) to try
to remove the same fd twice?

> The only other case would be some fd which has a NUL s->pe from the
> beginning, but then the existing code would have also crashed though
> only after calling apr_pollset_remove(). So I guess that case is not
> possible.

+1

What you (Mark) have done is just avoid a seg fault when s->pe is NULL.
What you haven't done (which might not actually be necessary) is solve
any issue where this method shouldn't be called in the first place (i.e.
state management).

-chris

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to