On Wed, May 20, 2009 at 12:05 AM, David Brownell <[email protected]> wrote: >> patch 02 renames a local variable 'x' into 'num_taps' to describe what it >> means > > Except I don't like seeing declaration blocks split into > sections with whitespace: > >> { >> jtag_tap_t *tap; >> - int x; >> int nth_tap; >> int scan_size = 0; >> >> + int num_taps = jtag_NumEnabledTaps(); > > Just put the "int num_taps..." up where "int x" was.
The block above num_taps is gone in the final version. jtag_NumEnabledTaps() is not a trivial initialization but rather a step of the algorithm. That's why it is set apart from the variable declarations. >> + >> /* allocate memory for a new list member */ >> jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t)); > > Or what you did in a few other places, "type * ptrvar" should > be just "type *ptrvar"; those are IMO neither simplifications The "* ptrvar" was added to enable the modification in #5 that you seemed to agree with. I personally prefer the syntax with spaces as I generally add spaces around anything that looks like an operator for better readability. > nor cleanups. Ditto using the (IMO annoying) C99 feature where > declarations don't necessarily live at the beginning of a block. Why do you need them at the beginning of the block, considering that they - are not used at that point - use up an extra statement - separate initialization from definition thus adding potential for errors and reducing readability. >> Again none of these patches change any algorithm. > > Making them all coding style fixes, not simplifications. > And at one point some new comments. Simple to read and simple to modify. That was the goal here. The point was that jtag.c contains utterly trivial algorithms that are hidden in bloated code. > I liked #5 (other than the "type * ptrvar" issue), that does > make the code more clear: p->q->r ==> q->r is mostly better. > > > I'd kind of like to see a standard "indent" ruleset agreed on, > so that at least the things that tool handles well could be > somewhat automated. > > > > (By the way, suggestion: only one patch per mail. It's painful > enough to try reviewing attachments, especially text/plain ones > that won't get the syntax highlighting that diffs do.) I don't think flooding the list with 10 mails is a good idea, especially if people start replying to the mails randomly. What extension will make your reader highlight the code? Michael _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
