command line string parsing / pointer confusion

[rhboarder]

i have a small app to be compiled into a .lib file. it accepts a char * array
and the array count, outputs a '-' and then reads in a line and (supposed) to
parse it into the recieved char * arrays. i know im doing something wrong
with writing to the memory locations but i cant figure it out. heres the code:

const int MAX_CHARS = 255;

extern "C"
MYAPI void getArgs(int argC, char * argV[])
{
	char holder[MAX_CHARS];

	cout << "-";
	cin.getline(holder, MAX_CHARS-1);
	strcat(holder, "\0");
	int index=0; //array index
                nBytes=1; //#of bytes to copy
                pos=0; //placeholder in holder string
	int chr;
	for (int i = 0; i < (int)strlen(holder)+1; i++)
	{
		chr = (int)*(char*)&holder[i];
		if (chr == 32 || chr == 0) //space or end string
		{
			if (index < argC)
			{
                                //copy nBytes-1, (nbytes is the string + the 
                                //space char or end string
				memcpy(&argV[index], &holder[pos], nBytes-1);
				pos = i+1;	//skip whitespace in $ holder
				index++;
			} else break;
			nBytes = 1;
		} else nBytes++;
	}
}
#############
//in the host application that calls that
const int MAX_ARGS = 4;
char * args[MAX_ARGS];
getArgs(2, args); //is it ok to use a number smaller than the
                 //actual # char* arrays?

[Arjay]

From msdn:

void *memcpy(void *dest, const void *src, size_t count);

Having the source and dest locations backwards is at least one of the problems.

[rhboarder]

its not backwords, i want to copy from the 'holder' string to the char * array 'argV[]'

[Arjay]

You already have the cmd params in argV, would would you need to copy them in there?

[Paul McKenzie]

memcpy(&argV[index], &holder[pos], nBytes-1);

Why are you overwriting memory that you never allocated or created? I'm speaking specifically of the argV array that is passed to you in main().

Regards,

Paul McKenzie

[rhboarder]

to arjay, i should have explained better, argV is blank/null, im wanting to fill them in through a function call that reads a line from stdin and parses it into (argV[0], argV[1],etc..)

your right paul i didnt allocate the memory for those, thanks for pointing that out.
now it works nicely, oh yeah i also had this wrong:

memcpy(&argV[index],
//changed to
memcpy(&argV[index][0]

works great now!

[GCDEF]

FWIW
for (int i = 0; i < (int)strlen(holder)+1; i++)
is pretty inefficient as strlen has to be calculated each iteration of the loop. Call strlen once and use the result in your loop.

[Arjay]

to arjay, i should have explained better, argV is blank/null, im wanting to fill them in through a function call that reads a line from stdin and parses it into (argV[0], argV[1],etc..)

I'm still not sure why you are using argV at all when there isn't any command line params to begin with. Plus since you didn't allocate the argV memory and there weren't any command line parameters, how do you know if argV has been allocated or is safe to write to? Generally argV memory should be considered read only.

[Paul McKenzie]

to arjay, i should have explained better, argV is blank/null, im wanting to fill them in

You should have getArgs return to you a char** that is allocated by getArgs instead of the caller attempting to create this array. In addition, another function should be written to destroy the argV array once it is not needed.

Here is some code, not tested or compiled, that first creates a vector of the arguments, and then creates a char** from the vector. Again, this has not been tested, but it should give you some idea of how to write your code more cleanly without the overuse of 'C' functions such as memcpy().

#include <string>
#include <vector>

using namespace std;

typedef std::vector<std::string> StringVector;
extern "C" MYAPI
char** getArgs(int argC, int*numArgsParsed)
{
    string holder;   
    StringVector argVector;
    getline(cin, holder);
    string::size_type startPos = 0;
    string::size_type endPos;
    int argCnt = 0;
    while ( argCnt < argC )
    {
         endPos = holder.find_first_of(' ', startPos);
         if (endPos != string::npos )
         {
            argVector.push_back(holder.substr(startPos, endPos));
            startPos = endPos + 1;
            ++argCnt;
         }          
         else
          {
               argVector.push_back(holder.substr(startPos));
               break;
         }
    }

    // Convert the vector to a char** argv.
    char** argV =  new char*[argVector.size()];
    for (int i = 0; i < argVector.size(); ++i)
    {
        argV[i] = new char [argVector[i].size() + 1];
        strcpy(argV[i], argVector[i].c_str());
    }
    *numArgsParsed = argVector.size();
    return argV;
}
//...
const int MAX_ARGS = 4;
int numArgs;
char ** args = getArgs(2, &numArgs);

extern "C"
MYAPI void DestroyArgs(char **argv, int nArgs)
{
        for (int i = 0; i < nargs; ++i)
        delete [] argv[i];
    delete [] argv;
}

Once again, not tested or compiled. The function getArgs returns to you the fully created char** -- the caller passes the maximum number of arguments, and a pointer to an int that gets filled with the actual number of arguments that were parsed in getArgs.

Then you call DestroyArgs() when you are done with the argv array.

Regards,

Paul McKenzie