Note newer patch attached. This is made directly against current git,
use instead of original patch.

On Wed, Nov 17, 2010 at 10:44 AM, Andrew Leech <coronasen...@gmail.com> wrote:
> On Wed, Nov 17, 2010 at 10:11 AM, Peter Stuge <pe...@stuge.se> wrote:
>
>>>  COMMAND_HANDLER(handle_svf_command)
>>>  {
>>> -#define SVF_NUM_OF_OPTIONS                   1
>>> +#define SVF_NUM_OF_OPTIONS                   2
>>
>> Aren't there three?
>>
> Yeah actually I had wondered about this, I simply incremented it....
> and didn't come back to check it. It should probably be 3 and fix the
> (1 + SVF_NUM_OF_OPTIONS) below.
>

Fixed.

>>
>>
>> This is somewhat heuristic:y however. Not nice. A more reliable
>> approach might be:
>>
>> usage: svf [-tap device.tap] <file> [quiet]
>>
>> In particular I would also like to avoid "device#" because the
>> parameter should not be a number at all, rather the dotted tap name.
>> You mentioned that you modelled after xsvf, so that help text should
>> probably also be clarified.
>>
>

Yep I rewrote the command line parser, so the usage is now:
"usage: svf [-tap device.tap] [-quiet] <file>"

Arguments are checked in a loop so it will accept switches and <file>
in any order. quiet or -quiet are both recognised. And of course, -tap
is optional. As such it is now back compatible, much nicer and more
flexible. There's also a help switch that simply prints usage in case
anyone tries it.

>>
>> Would it make sense to reuse the existing handling of head/tail
>> rather than creating commands to be run? This would remove the
>> need for a command buffer at all, and maybe make the change even
>> smaller.
>>
>
> Yeah I was initially going to do this, it's just that I found the code
> somewhat obfuscated and decided to go the quick and easy to read route
> rather than delving into the code much deeper. Basically it came to
> lazyness which is the ideal way to create code bloat....
> But yeah looking at it again now it does seem simpler than when I
> initially looked though the code, amazing how a little familiarity
> goes a long way. Perhaps I will fix it up, I've got a little time at
> the moment and looks like maybe it'll be quite straightforward.
>

I dug deep enough to figure out how it works, it uses array's that it
dynamically sets the size of, so it's a little convoluted to set them.
It's debatable whether this newer code is neater to understand, but it
is certainly far more efficient to run, and avoids all the extra
string handling.

Other than that it seems to run fine, I've tested the command line in
pretty much every direction and it works well. The operation of
header/trailer padding certainly seems to be completely compatible
with my first effort, I don't think I've missed anything of left out
any important steps.

Andrew

Attachment: svf_chain_2.patch
Description: Binary data

_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to