[gPXE-devel] [PATCH] [hci] Modify 'set' command to allow space separated values

[Marty Connor]

Hi Aaron,

Sorry for the delay.  Your patch seems useful, thanks for sharing it.

Does anyone have thoughts on Aaron's patch?  It seems like a useful
addition to me.  Can anyone think of potential problems that supporting
this addition could cause?

Thanks,

/ Marty /

On 9/20/10 11:49 PM, Aaron Brooks wrote:
> Just a ping to see if anyone has any comments or feedback. :-)
> 
> On Sat, Sep 18, 2010 at 9:12 AM, Aaron Brooks <aaron at brooks1.net> wrote:
>> It appears that GMail converted spaces to tabs even though I
>> specifically checked that the textbox contained tabs before I sent.
>> Sorry about the annoyance. I'm attaching an identical (albeit tabbed)
>> version of the patch.
>>
>> Sincerely,
>>
>> Aaron
>>
>> On Fri, Sep 17, 2010 at 6:05 PM, Aaron Brooks <aaron at brooks1.net> wrote:
>>> At our site we want to be able to store a series of values from a
>>> single DHCP option (in our case, kernel arguments) in this manner:
>>>
>>>    set kargs ${net0/224:string}
>>>
>>> I'm including our patch which allows multiple value arguments to the
>>> 'set' command. This change does not break or affect existing 'set'
>>> usage but will now make it legal to store space concatenated values.
>>>
>>> Let me know if you would like me to change any aspects of the implementation.
>>>
>>> I've fully tested the functionality including both error paths.
>>>
>>> Sincerely,
>>>
>>> Aaron
>>>
>>> -------------
>>> From: Aaron Brooks <aaron at brooks1.net>
>>> Date: Sun, 12 Sep 2010 22:53:45 -0400
>>> Subject: [PATCH] [hci] Modify 'set' command to allow space separated values
>>>
>>> The 'set' command previously assigned an identifier with the single
>>> value of the following positional argument. The following would assign
>>> the value of "bar" to the identifier of "foo"
>>>
>>>    set foo bar
>>>    show foo
>>>    foo = bar
>>>
>>> Providing additional positional arguments would lead to an error.
>>>
>>> The modification allows multiple positional arguments (either direct
>>> or via identifier expansion) to be concatenated with spaces and
>>> assigned as a single value to the declared identifier "foo":
>>>
>>>    set foo bar baz quux
>>>    show foo
>>>    foo = bar baz quux
>>>
>>> This functionallity was added to allow for multiple, space separated
>>> values (such as kernel arguments, passed via DHCP) to be assigned to an
>>> identifier.
>>>
>>> Signed-off-by: Aaron Brooks <aaron at brooks1.net>
>>> ---
>>>  src/hci/commands/nvo_cmd.c |   41 +++++++++++++++++++++++++++++++++++++----
>>>  1 files changed, 37 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/hci/commands/nvo_cmd.c b/src/hci/commands/nvo_cmd.c
>>> index 5eb2f06..ac18ee3 100644
>>> --- a/src/hci/commands/nvo_cmd.c
>>> +++ b/src/hci/commands/nvo_cmd.c
>>> @@ -30,19 +30,52 @@ static int show_exec ( int argc, char **argv ) {
>>>  }
>>>
>>>  static int set_exec ( int argc, char **argv ) {
>>> +       char *buf;
>>> +       int len = 0;
>>> +       int ofs = 0;
>>> +       int i;
>>>        int rc;
>>>
>>> -       if ( argc != 3 ) {
>>> -               printf ( "Syntax: %s <identifier> <value>\n", argv[0] );
>>> +       if ( argc < 3 ) {
>>> +               printf ( "Syntax: %s <identifier> <value>...\n", argv[0] );
>>>                return 1;
>>>        }
>>>
>>> -       if ( ( rc = storef_named_setting ( argv[1], argv[2] ) ) != 0 ) {
>>> +       /* Compute length of buffer */
>>> +       for ( i = 2; i < argc; i++ ) {
>>> +               len += strlen ( argv[i] );
>>> +               /* For joining space or trailing null */
>>> +               len += 1;
>>> +       }
>>> +
>>> +       buf = malloc ( len );
>>> +       if ( ! buf ) {
>>> +               const char *emsg = strerror ( errno );
>>> +               printf ( "Could not allocate memory to set \"%s\"=\"", argv[1] );
>>> +               for ( i = 2; i < (argc - 1); i++ ) {
>>> +                       printf ( "%s ", argv[i] );
>>> +               }
>>> +               printf ( "%s\": %s\n", argv[i], emsg );
>>> +               return 1;
>>> +       }
>>> +
>>> +       /* Join arguments into buffer with spaces */
>>> +       for ( i = 2; i < argc; i++ ) {
>>> +               strcpy ( buf + ofs, argv[i] );
>>> +               ofs += strlen ( argv[i] );
>>> +               buf[ofs] = ' ';
>>> +               ofs += 1;
>>> +       }
>>> +       buf[ofs - 1] = '\0';
>>> +
>>> +       if ( ( rc = storef_named_setting ( argv[1], buf ) ) != 0 ) {
>>>                printf ( "Could not set \"%s\"=\"%s\": %s\n",
>>> -                        argv[1], argv[2], strerror ( rc ) );
>>> +                        argv[1], buf, strerror ( rc ) );
>>> +               free ( buf );
>>>                return 1;
>>>        }
>>>
>>> +       free ( buf );
>>>        return 0;
>>>  }
>>>
>>> --
>>> 1.7.0.4
>>>
>>
> _______________________________________________
> gPXE-devel mailing list
> gPXE-devel at etherboot.org
> http://etherboot.org/mailman/listinfo/gpxe-devel