On Sun, May 17, 2009 at 10:54 PM, Rick Altherr <kc8...@kc8apf.net> wrote: > > On May 17, 2009, at 1:43 PM, David Brownell wrote: > >> On Sunday 17 May 2009, Michael Bruck wrote: >>> >>> - jtag_command_t **last_cmd; >>> - last_cmd = jtag_get_last_command_p(); >>> - >>> - *last_cmd = cmd_queue_alloc(sizeof(jtag_command_t)); >>> - (*last_cmd)->next = NULL; >>> - last_comand_pointer = &((*last_cmd)->next); >>> - (*last_cmd)->type = JTAG_SCAN; >>> >>> + jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t)); >>> + >>> + jtag_queue_command(cmd); >>> + >>> + cmd->type = JTAG_SCAN; >> >> Seems like a goodly fix ... but, couldn't all of those be wrapped >> up in a single function sort of like >> >> cmd = jtag_alloc_and_queue(JTAG_SCAN); >> >> Or STATEMOVE, PATHMOVE, etc. Agreed that queue_command() logic >> is exactly the error-prone stuff that really *needs* encapsulation; >> it should probably stay separate even with an alloc_and_queue().
I did not primarily want to compact code but separate layers. The function wraps the queue manipulation. The data structure initialization itself is much more code than just the type field so I don't like the idea of tearing it apart. >> _______________________________________________ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development > > > Rather than combine them, I'd like to see jtag_queue_command() enforce > validation of the command to be enqueued. Then the patterns would be: > > cmd = cmd_queue_alloc(); > > cmd->type = JTAG_SCAN; > .... > > jtag_queue_command(cmd); > > Otherwise, you place a command in the queue before it is filled out. This > works fine today where the JTAG queue is manually flushed, but if we ever > went to a opportunistic queue draining scheme (device driver pulls next item > off queue automatically when a command finishes), you could get bogus > commands being executed. > While I agree in principle I would like to get this patch through first and discuss this suggestion later. My patches are purely code restructuring that should *not* change the algorithms at all. Michael _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
