Hi,

On 08/04/2012 10:59 AM, Hans de Goede wrote:
> Note I did find one more issue wrt to timer handling, but that is unrelated
> (and in another part of the code), so lets just push this patch and then fix
> that issue with a separate patch. See my next mail on that.

Scrap that, upon typing a detailed mail on this and *almost* hitting send
I finally saw what is going on, libusb/io.c : handle_events() has both
an r and a ret local variable and it stores the result of an
arm_timerfd_for_next_timeout() call into *ret*, and then does things based
on *r* == 1, which was confusing me into thinking it did stuff depending on
the return value of arm_timerfd_for_next_timeout() being 1 versus 0, but that
is not true. So false alarm.

Regards,

Hans




>
> Regards,
>
> Hans
>
>> On 2012.08.03 10:13, Hans de Goede wrote:
>>> That works for me, I agree the original code is hard to read, maybe move
>>> the refactoring to a separate patch though ?
>>
>> Well, I don't really see it as refactoring, as we're moving a section of 
>> code that used first into a call that didn't use first, so we're simply 
>> adding the first variable there.
>> I have also now added a check for timerisset in the code moved to 
>> add_to_flying_list(), which simplifies things further.
>>
>>> Hehe, funny I had the exact same thought when reviewing your patch, the
>>> problem is not with the code, but with the confusing list_add_tail function
>>> name, here is docs on list_add_tail:
>>>
>>> http://www.compsoc.man.ac.uk/~moz/kernelnewbies/documents/kdoc/kernel-api/r509.html
>>>
>>>
>>> "Insert a new entry before the specified head. This is useful for
>>> implementing queues. "
>>>
>>> So the original code is fine here.
>>
>> Agreed. I was trying too hard to see bugs where there weren't any.
>>
>>> I've a slight preference for moving the check outside of the locked
>>> context,
>>> since I prefer always having lock and unlock calls balanced 1:1
>>
>> That's a good rationale, so I went with that too.
>>
>> Regards,
>>
>> /Pete
>>
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>>
>>
>>
>> _______________________________________________
>> libusbx-devel mailing list
>> libusbx-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/libusbx-devel
>>

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to