[ofa-general] Re: [IBSIM] Parse sim cmds by name not first character

[Al Chu]

Hey Sasha,

On Mon, 2008-08-18 at 23:39 +0300, Sasha Khapyorsky wrote:
> Hi Al,
> 
> On 14:07 Wed 13 Aug     , Al Chu wrote:
> > 
> > I was looking into adding a new command to ibsim,
> 
> Cool :)
> 
> > but since the original
> > cmd-parsing function only checks for the first char of the inputted
> > command, it limits the ability to add a reasonable-sounding new command
> > name.  The patch changes the function to check the entire command name.
> 
> This would break a lot of existing test scripts, so basically I disagree
> with such radical change. However we could do something less destructive
> and achieve your goal - lets compare string partially as provided:
>
>    strncasecmp(line, "Dump", strlen(line))
> 
> (or 'strncasecmp(line, "Dump", cmd_len)' if you don't want to put '\0'
> into line buffer)

I don't quite understand.  How would my patch break existing scripts?

Do the existing test scripts not have whitespace between the command and
the options?  If that's the case, then we could do as you suggested and
program in:

        if (!strncasecmp(line, "Dump", 4))
                r = dump_net(f, line);
        else if (!strncasecmp(line, "Route", 5))
                r = dump_route(f, line);

as you suggested.

Or do the existing test scripts only use the first character of the
command?  If that's the case, then I guess we could program in single
character commands to be legacy-special cases.  But we would require
whitespace between the command and options for that to work.

> Looks fine?
> 
> Also some comment is below.
> 
> [snip...]
> 
> > From 5871b81d1ebdf86f9a9fcf79c8d8a558fd2600b1 Mon Sep 17 00:00:00 2001
> > From: Albert Chu <chu11 at llnl.gov>
> > Date: Wed, 13 Aug 2008 13:53:14 -0700
> > Subject: [PATCH] parse sim cmds via full name
> > 
> > 
> > Signed-off-by: Albert Chu <chu11 at llnl.gov>
> > ---
> >  ibsim/sim_cmd.c |  105 +++++++++++++++++++++----------------------------------
> >  1 files changed, 40 insertions(+), 65 deletions(-)
> > 
> > diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
> > index 1f6ba88..a35d0f4 100644
> > --- a/ibsim/sim_cmd.c
> > +++ b/ibsim/sim_cmd.c
> > @@ -757,91 +757,66 @@ int netstarted = 0;
> >  
> >  int do_cmd(char *buf, FILE *f)
> >  {
> > +	char cmdbuf[4096];
> 
> Why this huge buffer? 

I just copied the bufsize from the caller of do_cmd(), so I wanted to be
consistent for a potentially uber-long command string.  Naturally, we
don't need to do it this way.

> Actually we don't need to copy there just find
> appropriate length for comparison, no?

I did it primarily for the output of the bad command output message at
the end.  I guess we could stick a '\0' into the original linebuf.  But
I didn't want to edit the buffer and change whatever parsing
expectations the other functions had.

Al

> Sasha
-- 
Albert Chu
chu11 at llnl.gov
925-422-5311
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory