On Thu, Jun 9, 2011 at 7:29 AM, Peter Stuge <[email protected]> wrote:
> Øyvind Harboe wrote:
>> Nit, the construct below is a little bit fancy. I'd prefer splitting
>> it over multiple lines to make it more accessible to the
>> casual reader.
>
> Or just use goto, please, which is clear, simple, well understood,
> and elegant all at once. :)

I think goto is OK sometimes. When it is used to unwind the stack
frame and free up resources, in lieu of exceptions and resource
tracking it can clean up the code significantly.

The patch uses code duplication and if statements inside the
cleanup fn(if (buffer) free(buffer)), which can be avoided using
exception handling and resource tracking. In the case of this patch,
it looks like the if (ft2232_buffer) free(ft2232_buffer) is a case
of "conservative programming". I'd much rather have an assert
in this case than to have broken code "auto-repair" and be harder
to get right.

Note that I'm not holding off committing the patch for the above, as
maintainer I'm interested in progress as well as more philosophical coding
style discussions. :-)

If someone does come forth with an even more cleaner patch, then
I'm OK with that. This is now growing into a case-study of how to formulate
a patch more than a fix. The openocd community has much to learn
about how to formulate patches.

As long as there is progress, I don't mind holding off the patch.


-- 
Øyvind Harboe

Can Zylin Consulting help on your project?

US toll free 1-866-980-3434 / International +47 51 87 40 27

http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to