On Wed, Jul 8, 2009 at 12:15 PM, Robert P. J. Day<rpjday-L09J2beyid0N/H6P543EQg at public.gmane.org> wrote:
> On Wed, 8 Jul 2009, D. Hugh Redelmeier wrote:
>
>> | From: Robert P. J. Day <rpjday-L09J2beyid0N/H6P543EQg at public.gmane.org>
>>
>> | http://cli.gs/0LUN27
>>
>> int err;
>> /* registration takes a pointer and a name */
>> err = register_this(ptr1, "skull");
>> if (err) goto fail_this;
>> err = register_that(ptr2, "skull");
>> if (err) goto fail_that;
>> err = register_those(ptr3, "skull");
>> if (err) goto fail_those;
>> return 0; /* success */
>>
>> fail_those: unregister_that(ptr2, "skull");
>> fail_that: unregister_this(ptr1, "skull");
>> fail_this: return err; /* propagate the error */
>>
>> I think that the code would be clearer if written this way:
>>
>> int err;
>> /* registration takes a pointer and a name */
>> err = register_this(ptr1, "skull");
>> if (!err) {
>> err = register_that(ptr2, "skull");
>> if (!err) {
>> err = register_those(ptr3, "skull");
>> if (!err)
>> return 0; /* success */
>>
>> unregister_that(ptr2, "skull");
>> }
>> unregister_this(ptr1, "skull");
>> }
>> return err; /* propagate the error */
>
> i disagree, and there's actually a tradition in modules to use the
> "goto" to avoid precisely that cascading indentation. in any event, i
> was stealing straight out of the LDD3 book so i wanted to stick with
> their content to allow readers to cross-reference and not have to
> mentally translate.
>
> but even if that last part weren't an issue, i'd probably still go
> with the original code. it seems to be the standard and i find it
> more readable. but that's just me.
>
> rday
one thing that took me time and one eventually comes to this
conclusion as a developer is to follow the standard on the project
they are coding for. This helps to keep the format the same and makes
for more maintainable code. if someone knows the structure and format
of coding policy, then they don't need to stop and think when reading
your new code.
i.e. some functions will return 0 for success, while others will
return a non-zero for success, that's what happen when you don't stop
to see how other before you are doing things.
I personally do not like goto statements, but I agree with rday, it's
much easier to read code that is not nested. If that is the way the
kernel developer are doing things (using goto), then I would comply.
if kernel developers are doing core review I am sure they would point
certain things out and ask the submitter to modified before
acceptance? rday would know about that better than me since I've never
participate on a open source project myself.
--
Kind Regards,
Rajinder Yadav
--
The Toronto Linux Users Group. Meetings: http://gtalug.org/
TLUG requests: Linux topics, No HTML, wrap text below 80 columns
How to UNSUBSCRIBE: http://gtalug.org/wiki/Mailing_lists