Two-Dimensional Char Array

[miketigerwoods]

I'm having problems where I need to parse a command line, and place
each token from strtok() into an array. I've read here:
http://www.phim.unibe.ch/comp_doc/c_manual/C/CONCEPT/arrays.html (at
bottom) that you can access a char[][] by doing char and getting the
string. However when I try the code below I get

error: invalid use of array with unspecified bounds

whenever argv is accessed. It is declared in main() as:

char argv[ARGV_LENGTH][ARGV_LENGTH]; /* ARGV_LENGTH is a const int of
64 */

This is pretty much my first time with C, I'm usually using Java and a
little C++. Any help is appreciated.

Thanks,
Mike


void parse_cmdline(char cmdline [], char argv [][])
{
char * command;
int i = 1;
command = strtok(cmdline, DELIMITERS);
argv[0] = malloc(sizeof(char)*(strlen(command)+1));
strcpy(argv[0],command);
while (command != NULL) {
printf("%s\n", command);
command = strtok(NULL, DELIMITERS);
argv = malloc(sizeof(char)*(strlen(command)+1));
strcpy(argv,command);
i++;
}
}

 

[Vimal Aravindashan]

I'm having problems where I need to parse a command line, and place
each token from strtok() into an array. I've read here:
http://www.phim.unibe.ch/comp_doc/c_manual/C/CONCEPT/arrays.html (at
bottom) that you can access a char[][] by doing char and getting the
string. However when I try the code below I get

You need to read something better. I suggest K&R2

error: invalid use of array with unspecified bounds

whenever argv is accessed. It is declared in main() as:

char argv[ARGV_LENGTH][ARGV_LENGTH]; /* ARGV_LENGTH is a const int of
64 */

Please post the code where you call your parse_cmdline() function.

void parse_cmdline(char cmdline [], char argv [][])

When passing unsized arrays, you can optionally leave out only the
leftmost dimension. The remaining must be specified.

{
char * command;
int i = 1;
command = strtok(cmdline, DELIMITERS);
argv[0] = malloc(sizeof(char)*(strlen(command)+1));

You passed a two dimensional array to this function! Maybe what you
really wanted to pass was an array of pointers.

strcpy(argv[0],command);
while (command != NULL) {
printf("%s\n", command);
command = strtok(NULL, DELIMITERS);
argv = malloc(sizeof(char)*(strlen(command)+1));
strcpy(argv,command);

You are wasting a call when command equals NULL in the end. Use a
do...while instead.

i++;
}
}

Hope this helps,
Vimal.

 

[miketigerwoods]

The spec for this assignment shows a two-dimensional array, no mention
of pointers anywhere. As for the website vs. the mentioned text, I have
a C++ text that I use, I really just never use C so it's just not worth
it in my opinion. Those do...while's always slip my mind, thanks for
that.

Thanks,
Mike

int main(void)
{
char cmdline[MAX_CMD_LENGTH];
char argv[ARGV_LENGTH][ARGV_LENGTH];
while (TRUE) {
display_prompt();
get_user_input(cmdline);
parse_cmdline(cmdline, argv);
if (!try_builtins(argv))
fork_and_execute(argv);
}
exit(0); /* should never reach here */
}

 

[Vimal Aravindashan]

The spec for this assignment shows a two-dimensional array, no mention
of pointers anywhere. As for the website vs. the mentioned text, I have
a C++ text that I use, I really just never use C so it's just not worth
it in my opinion. Those do...while's always slip my mind, thanks for
that.

Irrespective of whether you use C or C++, you cannot re-assign an
address of an element in an array:

int parse_cmdline(char *cmdline, char argv[][64])
{
int i;

i = 0;
cmdline = strtok(cmdline, DELIMITERS);
do {
strcpy(argv[i++], cmdline);
cmdline = strtok(NULL, DELIMITERS);
} while(cmdline != NULL);

return i;
}

Cheers,
Vimal.

 

[Neil Cerutti]

I'm having problems where I need to parse a command line, and place
each token from strtok() into an array. I've read here:
http://www.phim.unibe.ch/comp_doc/c_manual/C/CONCEPT/arrays.html (at
bottom) that you can access a char[][] by doing char and getting the
string. However when I try the code below I get

error: invalid use of array with unspecified bounds

whenever argv is accessed. It is declared in main() as:

char argv[ARGV_LENGTH][ARGV_LENGTH]; /* ARGV_LENGTH is a const int of
64 */

This is pretty much my first time with C, I'm usually using Java and a
little C++. Any help is appreciated.

Thanks,
Mike


void parse_cmdline(char cmdline [], char argv [][])
{

You cannot use unspecified size arrays in C.

In C,

/* 1 */ void f(int a[])

is synonymous with

/* 2 */ void f(int *a)

which is synonymous with

/* 3 */ void f(int a[50000])

They all declare that parameter a is a pointer to int.

Thus, what you have really declared is this:

void parse_cmdline(char *cmdline, char (*argv)[])

Parameter argv is a pointer to an unspecied size array of char.
All arrays must have a specified size, so your code is an error.
Without knowing the size of the object you are storing in each
element of argv, C has no way to compute indexes, e.g., argv[1].

You need this:

void parse_cmdline(char cmdline[], char[][ARGV_LENGTH])
{

or this:

void parse_cmdline(char *cmdline, char (*argv)[ARGV_LENGTH])
{

Some programmers prefer to use the [] syntactic sugar when they
plan to treat the parameter as an array, and the pointer syntax
when they plan to treat it as a pointer. I always use the pointer
syntax, since otherwise I feel like I'm lying to myself.

 

[Christopher Benson-Manica]

I'm having problems where I need to parse a command line, and place
each token from strtok() into an array. I've read here:
http://www.phim.unibe.ch/comp_doc/c_manual/C/CONCEPT/arrays.html

Don't read there any more. The page dealing with NULL is proof enough
that the site's authors are not to be trusted. They don't seem
particularly interested in so simple a thing as the distinction
between the NULL macro and the null character, among other unfortunate
misstatements.

 

[Netocrat]

]

int parse_cmdline(char *cmdline, char argv[][64])

64 would be better as the OP-defined macro ARGV_LENGTH.

{
int i;

i = 0;
cmdline = strtok(cmdline, DELIMITERS);

The original while loop had the correct semantics. This do loop fails to
check for the above function returning NULL. There is nothing wasted - it
is a necessary check.

do {

while(cmdline != NULL && i < ARGV_LENGTH) {

strcpy(argv[i++], cmdline);
cmdline = strtok(NULL, DELIMITERS);

You have perpetuated the OP's use of strtok, and it should be mentioned
that this is in general a less than ideal function for many reasons,
documented in plenty of places (e.g. this group's archives).

} while(cmdline != NULL);

/* return value >= ARGV_LENGTH indicates under-sized buffer */

 

[Vimal Aravindashan]

]

int parse_cmdline(char *cmdline, char argv[][64])

64 would be better as the OP-defined macro ARGV_LENGTH.

{
int i;

i = 0;
cmdline = strtok(cmdline, DELIMITERS);

The original while loop had the correct semantics. This do loop fails to
check for the above function returning NULL. There is nothing wasted - it
is a necessary check.

Check the OP's code again. He doesn't check for the first one himself.
In most real world scenarios, there are input specifications. Some of
which favour the programmer. Like in this case, the OP maybe guaranteed
that 'cmdline' parameter has one or more tokens, and also less than 64
(ARGV_LENGTH). Given the OP's code, I think its safe to assume further
that his set of DELIMITERS combined with strtok() will yield the
necessary results for him.

do {

while(cmdline != NULL && i < ARGV_LENGTH) {

strcpy(argv[i++], cmdline);
cmdline = strtok(NULL, DELIMITERS);

You have perpetuated the OP's use of strtok, and it should be mentioned
that this is in general a less than ideal function for many reasons,
documented in plenty of places (e.g. this group's archives).

Above comments apply here too.

/* return value >= ARGV_LENGTH indicates under-sized buffer */

Cheers,
Vimal.

 

[Vimal Aravindashan]

Check the OP's code again. He doesn't check for the first one himself.
In most real world scenarios, there are input specifications. Some of
which favour the programmer. Like in this case, the OP maybe guaranteed
that 'cmdline' parameter has one or more tokens, and also less than 64
(ARGV_LENGTH). Given the OP's code, I think its safe to assume further
that his set of DELIMITERS combined with strtok() will yield the
necessary results for him.

Oh, and I forgot to mention that all I intended to show the OP was that
he was misusing malloc(). An error he would encounter once he cleared
"invalid use of array with unspecified bounds" error. ( 'misuse' may not
be the proper wording here, but I cant think of another ;-) )

Cheers,
Vimal.

 

[Netocrat]

On Tue, 18 Oct 2005 23:00:35 +0530, Vimal Aravindashan wrote: [...]

int parse_cmdline(char *cmdline, char argv[][64])

64 would be better as the OP-defined macro ARGV_LENGTH.

{
int i;

i = 0;
cmdline = strtok(cmdline, DELIMITERS);

The original while loop had the correct semantics. This do loop fails
to check for the above function returning NULL. There is nothing
wasted - it is a necessary check.

Check the OP's code again. He doesn't check for the first one himself.

Here is the relevant portion of the OP's code:

command = strtok(cmdline, DELIMITERS); ...
while (command != NULL) {
...
command = strtok(NULL, DELIMITERS);
...
}

This has correct semantics; the do loop does not.

Perhaps you misread my comments as applying to the test for i <
ARGV_LENGTH that I added.

In most real world scenarios, there are input specifications. Some of
which favour the programmer. Like in this case, the OP maybe guaranteed
that 'cmdline' parameter has one or more tokens, and also less than 64
(ARGV_LENGTH).

He may be, however there's no need to assume - the check is simple.

Passing the size of the array's first dimension as a parameter rather than
fixing it as ARGV_LENGTH is a possible extension, but this is probably
beyond the OP's requirements.

Given the OP's code, I think its safe to assume further that his set of
DELIMITERS combined with strtok() will yield the necessary results for
him.

It will likely generate correct results without ill-effect for the OP's
situation, but a general warning re strtok is warranted.

[...]

 

[Vimal Aravindashan]

Here is the relevant portion of the OP's code:

command = strtok(cmdline, DELIMITERS); ...
while (command != NULL) {
...
command = strtok(NULL, DELIMITERS);
...
}

This has correct semantics; the do loop does not.

Perhaps you misread my comments as applying to the test for i <
ARGV_LENGTH that I added.

I didn't misread it. Forget about what maybe relevant or not. The
following is this OP's original post:

command = strtok(cmdline, DELIMITERS);
argv[0] = malloc(sizeof(char)*(strlen(command)+1));
strcpy(argv[0],command);
while (command != NULL) {

As per the above code, even if strtok() returns NULL for the first call,
the OP makes it a point to call strcpy() for it.
Netocrat, I don't think either of us can decide what the "correct
semantics" are for the OP solution. I am surprised that he hasn't
commented on our replies, because apparently he wants a NULL at the end
of the arg list, which neither of our codes do.

It will likely generate correct results without ill-effect for the OP's
situation, but a general warning re strtok is warranted.

Mike, you've been warned.

Cheers,
Vimal.

 

[Netocrat]

Here is the relevant portion of the OP's code:

command = strtok(cmdline, DELIMITERS); ...
while (command != NULL) {
...
command = strtok(NULL, DELIMITERS);
...
}

This has correct semantics; the do loop does not.

Perhaps you misread my comments as applying to the test for i <
ARGV_LENGTH that I added.

I didn't misread it. Forget about what maybe relevant or not. The
following is this OP's original post:

command = strtok(cmdline, DELIMITERS);
argv[0] = malloc(sizeof(char)*(strlen(command)+1));
strcpy(argv[0],command);
while (command != NULL) {

As per the above code, even if strtok() returns NULL for the first call,
the OP makes it a point to call strcpy() for it.

He does and that is unsafe - strlen and strcpy do not take NULL pointers.
The original code's looping is not entirely correct but with those two
lines removed it is - perhaps my omitting them was misleading; the point
is that the while loop is more naturally suited to correctly dealing with
errors.

For the do loop to behave correctly one would have to either:
a) only enter the loop if cmdline != NULL - but why write the code for
that additional test when the while loop code already provides it?
b) eliminate the call to strtok prior to the loop, reverse the order of
the functions within the loop and break if strok returns NULL - but then
to remove unnecessary tests as you seemed to want to do, we should remove
the loop conditional - so a do loop is no more preferable than a
while(1){} or for(;{} loop and the code is considerably different to
that originally proposed.

Netocrat, I don't think either of us can decide what the "correct
semantics" are for the OP solution.

True, but we can decide when a solution fails to prevent NULL from being
passed to functions to which it should not be passed.

I am surprised that he hasn't commented on our replies, because
apparently he wants a NULL at the end of the arg list, which neither of
our codes do.

Well that can be added immediately prior to the return if he does want it:

if (i == ARGV_LENGTH)
i--;
argv[i++] = NULL;

[...]