Syslinux - Sep 2010 - [RFC] function to parse string to argc/argv pair

Currently, I'm looking to improve rosh and would like to start using
getopt().  I'd like to parse a string obtained by fgets() to be like
the argc/argv pair created by __parse_argv but in a way that's more
cross-platform (such that it can also be used by .lnx files rather
than just COM32) and suitable to use in rosh and lua.c32.

I've made a function and inlined a test program for the function.  It
concatenates all command line arguments to one string, prints its
length (per strlen) and the string then breaks it apart and prints the
argument number and the argument (zero-origin, quoted for clarity).

What I'm looking for is comments/suggestions/feedback on how I should
improve this.  I realize that it does not handle quoting at all (just
like __parse_argv) but I'd like to try to do that in another version.

-- 
-Gene


--------
/* ----------------------------------------------------------------------- *
 *
 *   Copyright 2010 Gene Cumm - All Rights Reserved
 *
 *   Portions from com32/lib/sys/argv.c:__parse_argv()
 *   Copyright 2004-2009 H. Peter Anvin - All Rights Reserved
 *   Copyright 2009 Intel Corporation; author: H. Peter Anvin
 *
 *   This program is free software; you can redistribute it and/or modify
 *   it under the terms of the GNU General Public License as published by
 *   the Free Software Foundation, Inc., 53 Temple Place Ste 330,
 *   Boston MA 02111-1307, USA; either version 2 of the License, or
 *   (at your option) any later version; incorporated herein by reference.
 *
 * ----------------------------------------------------------------------- */

/*
 * argtst.c
 *
 * testing of argument parser
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>		/* strcpy() strlen() memcpy() strchr() */
#include <errno.h>		/* errno; error macros */

#include <consoles.h>
#include <getkey.h>
#include <ctype.h>

#define INS	256


void print_argv(int argc, char *argv[])
{
    int i;
    for (i = 0; i < argc; i++) {
	printf("%4d '%s'\n", i, argv[i]);
    }
}

/*
 * parse_args1: Try 1 at parsing a string to an argc/argv pair.  use
free_args1 to free memory malloc'd
 *
 * Derived from com32/lib/sys/argv.c:__parse_argv()
 *   Copyright 2004-2009 H. Peter Anvin - All Rights Reserved
 *   Copyright 2009 Intel Corporation; author: H. Peter Anvin
 */
int parse_args1(char ***iargv, const char *istr)
{
    int argc  = 0;
    const char *p;
    char *q, *r, *args, **arg;
    int sp = 1;	//, qt = 0;		/* Was a space; inside a quote */

    /* Scan 1: Length */
    /* I could eliminate this if I knew a max length, like strncpy() */
    int len = strlen(istr);

    /* Scan 2: Copy, nullify and make argc */
    if (!(args = malloc(len + 1)))
	goto fail_args;
    q = args;
    for (p = istr;; p++) {
	if (*p <= ' ') {
	    if (!sp) {
		sp = 1;
		*q++ = '\0';
	    }
	} else {
	    if (sp) {
		argc++;
		sp = 0;
	    }
	    *q++ = *p;
	}
	if (!*p)
	    break;
    }

    q--;			/* Point q to final null */
    /* Scan 3: Build array of pointers */
    if (!(*iargv = malloc((argc + 1) * sizeof(char *))))
	goto fail_args_ptr;
    arg = *iargv;
    arg[argc] = NULL;		/* Nullify the last pointer */
    if (*args != '\0')
	    *arg++ = args;
    for (r = args; r < q ; r++) {
	if (*r == '\0') {
	    *arg++ = r + 1;
	}
    }

fail_args:
    return argc;
fail_args_ptr:
    free(args);
    return 0;
}

void free_args1(char ***argv)
{
    char *s;
    s = **argv;
    free(*argv);
    free(s);

}

/* Concatenate command line arguments into one string
 *	str	Output character array
 *	strl	Size of array
 *	argc	Argument Count
 *	argv	Argument Values
 *	returns	Length of output string (like strlen())
 */
int argcat(char *str, const int strl, const int argc, char *argv[])
{
    int i, arglen;			/* index, argument length */
    int curpos, maxlen = strl - 1;	/* current position in str */
    curpos = 0;
    str[0] = '\0';		/* Nullify string just to be sure */
    for (i = 0; i < argc; i++) {
	arglen = strlen(argv[i]);
	/* Theoretically, this should never be met in SYSLINUX */
	if ((curpos + arglen) > maxlen)
	    arglen = maxlen - curpos;
	memcpy(str + curpos, argv[i], arglen);
	curpos += arglen;
	if (curpos >= maxlen) {
	    /* Hopefully, curpos should not be greater than maxlen */
	    /* Still need a '\0' at the last character */
	    str[maxlen] = '\0';
	    break;		/* Escape out of the for() loop;
				   We can no longer process anything more */
	} else {
	    str[curpos] = ' ';
	    curpos += 1;
	    str[curpos] = '\0';
	}
    }
    /* If there's a ' ' at the end, remove it.  This is normal
unless
       the maximum length is met/exceeded. */
    if (str[curpos - 1] == ' ')
	str[--curpos] = '\0';
    return curpos;
}				/* rosh_argcat */

int main (int argc, char *argv[])
{
    char in[INS];
    int inl;
    char *ret;
    int argc2;
    char **argv2;
#ifdef __COM32__
    console_ansi_std();
#endif
    inl = argcat(in, INS, argc, argv);
    printf(" %d: ", inl);
    puts(in);
    argc2 = parse_args1(&argv2, in);
    print_argv(argc2, argv2);
    free_args1(&argv2);
    puts("");
    return 0;
}

On Thu, Sep 16, 2010 at 12:38, Gene Cumm <gene.cumm at gmail.com> wrote:

At this time, the code works but I'm looking for advise on which
alternatives I should use to optimize this function, especially in the
context of Syslinux/COM32.
> int parse_args1(char ***iargv, const char *istr)
Two contemplations here.

1) Pass a length argument at which point processing will stop which
could eliminate the need for the length scan.
2) To reuse the incoming array, eliminating the need to allocate it in
the parser (malloc, calloc, etc.).
> ? ?/* Scan 1: Length */
> ? ?/* Scan 2: Copy, nullify and make argc */
Pre-allocate an array for the full length (needing a length by scan or
argument; requires only 1 call to free()) or allocate multiple arrays
(one per argument; still requires a length scan or a lot of recopying
of the array).
> ? ?/* Scan 3: Build array of pointers */
Allocate the array of pointers after copying/nullifying the array or
take the conservative approach of assuming the maximum possible value
of argc ( (n+1)/2 ), which could allow the copying/nullifying and
filling of the pointers to be done at the same time (requiring only a
length scan and a copy scan).  Alternatively, pre-allocate a maximally
sized pointer array, fill the pointers while copying/nullifying then
allocate an optimally sized pointer array, copy active pointers and
free the large pointer array.
> void free_args1(char ***argv)
> {
> ? ?char *s;
> ? ?s = **argv;
> ? ?free(*argv);
> ? ?free(s);
The order here shouldn't matter as it's on the heap.  Reversing the
order would prevent the need to allocate a char * on the stack (so
small it shouldn't matter either).

-- 
-Gene