Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <3E15C46C.3050705@electricjellyfish.net>
Date: Fri, 03 Jan 2003 12:12:12 -0500
From: Garrett Rooney <rooneg@electricjellyfish.net>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
 rv:1.1) Gecko/20020826
MIME-Version: 1.0
To: Brian Pane <brian.pane@cnet.com>
CC: dev@apr.apache.org
Subject: Re: cvs commit: apr/poll/unix poll.c
References: <20021231171040.98637.qmail@icarus.apache.org>
	 <20030103160522.GC1366@redhat.com> <1041613127.6053.8.camel@desktop>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Brian Pane wrote:

>On Fri, 2003-01-03 at 08:05, Joe Orton wrote:
>  
>
>>On Tue, Dec 31, 2002 at 05:10:40PM -0000, William Rowe wrote:
>>    
>>
>>>  Modified:    poll/unix poll.c
>>>  Log:
>>>    Fix apr_poll behavior on Darwin/Win32 (now passing testpoll.)
>>>    We were getting spurious returned events because the select-based
>>>    poll implementation wasn't zeroing out previous results before
>>>    setting the current ones and returning.
>>>  
>>>  Submitted by:	Garrett Rooney <rooneg@electricjellyfish.net>
>>>  
>>>  Revision  Changes    Path
>>>  1.33      +4 -0      apr/poll/unix/poll.c
>>>  --- poll.c	20 Sep 2002 10:42:37 -0000	1.32
>>>  +++ poll.c	31 Dec 2002 17:10:40 -0000	1.33
>>>  @@ -275,6 +275,10 @@
>>>       }
>>>   #endif
>>>   
>>>  +    for (i = 0; i < *nsds; i++) {
>>>  +      aprset[i].rtnevents = 0;
>>>  +    }
>>>  +
>>>       (*nsds) = rv;
>>>      
>>>
>>I don't think this is correct; *nsds is a return value and isn't defined
>>until the following line, so this may cause i to overrun the array. (I'm
>>seeing memory corruption from this on some platforms)
>>    
>>
>
>Yeah, that looks bad.
>

good catch.  i should have noticed that.

>  
>
>>Even so, why is this change necessary, since the for loop at the end of
>>the function is doing exactly the same thing already?
>>    
>>
>
>If select returns zero (no descriptors signaled), the function
>will exit before it ever reaches that loop.  However, the
>"for (i = 0; i < *nsds; i++)" loop isn't right, either; even
>when *nsds is initialized, it's only equal to the number of
>descriptors that have been signaled, not the total number
>of descriptors in the input vector.  We need to loop to
>num instead of *nsds.
>  
>

yeah, that's the reason we need to clear it, and the way it should be done.

thanks,

-garrett