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
signature.asc
Description: OpenPGP digital signature