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).
Best regards, Wolfgang _______________________________________________ Xenomai mailing list Xenomai@xenomai.org http://www.xenomai.org/mailman/listinfo/xenomai
