Re: [Xenomai] [PATCH 1/2] Refactor ipipe_select_timers

[Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>]

On 09/28/2012 10:32 AM, Wolfgang Mauerer wrote:

> On 27/09/12 20:33, Gilles Chanteperdrix wrote:
>> On 09/27/2012 02:47 PM, Wolfgang Mauerer wrote:
>>
>>> On 27/09/12 14:04, Gilles Chanteperdrix wrote:
>>>> On 09/27/2012 10:28 AM, Wolfgang Mauerer wrote:
>>>>> On 26/09/12 23:28, Gilles Chanteperdrix wrote:
>>>>>> On 09/26/2012 03:16 PM, Wolfgang Mauerer wrote:
>>> (...)
>>>
>>>>>> Talking about readability, I find a goto with a clear label name much
>>>>>> more readable than a flag. So, NACK this patch, please keep the goto.
>>>>>  
>>>>> So you're against the refactoring, or only against using the flag?
>>>>> Keeping the goto leads to something like
>>>>>
>>>>> 	if (install_pcpu_timer(cpu, hrclock_freq, t))
>>>>> 		goto found
>>>>> (...)
>>>>> found:    ;
>>>>>
>>>>> since we need a statement for the label, but nothing is left to do.
>>>>> I find this fairly ugly, but if you prefer it over a flag, then
>>>>> so be it.
>>>>
>>>> Then use return instead of goto...
>>>
>>> Won't work -- that skips the rest of the enclosing per_cpu loop and
>>> the second part of the function introduced in the follow-up commit
>>> that does the actual bugfixing.
>>>
>>> Since I take the flag is the issue and not the refactoring as such, 
>>> please find an updated patch with a goto below.
>>
>>
>> Oh boy, you love functions, do you? What I would do is: keep the test
> sure() :)
>> for evtdev->mode outside of the install_pcpu_timer function so that we
>> clearly see in the loop that it is the only reason for continuing the
>> loop. Then use the goto found, and at the found label, call the now void
>> function install_pcpu_timer. Ok for you?
>>
> okay, changed the code as desired to introduce more labels and use the 
> preprocessor more often. Please pick 23b2de46314 and b738a3b624 from 
> https://github.com/siemens/ipipe core-3.5_for-upstream (apply also
> cleanly to core-4).


Ok. Applied to my for-core-4 branch, thanks.

-- 
                                                                Gilles.