C question: Returning string array from function

[john]

Relatively new to C coding, so any help would greatly be appreciated.
I'm having problems try to return my string array from my parsing
function. When I do a printf I am getting the correct value for my first
element but my subsequent printf's will return garbage. Can someone let
me know what I am doing wrong. Thanks in advance.

-jlewis



typedef char wrkspace[40][200]; <- Locatated in my haeder


int open_file (char *path, char *recNo, int fid) {
FILE *fp;
char str [1000];
int ret;
int count;
int inx;
int i;
wrkspace *value;

if ((fp=fopen(path, "r"))==NULL) {
printf("Cannot open file.\n");
return -1;
}

count = atoi(recNo);
memset (str, NULL, 1000);
fgets( str, sizeof(str), fp );

while (!feof(fp)) {
for (inx = 0; (inx < count ) && (!feof(fp)) && ( strlen(str) > 0
); inx++) {
<--CALLING ROUTINE --->
value = (wrkspace *) parse_record(&str, fid, count);

printf ("0:%s\n", inx, value[0]); <-- returns good data
printf ("1:%s\n", inx, value[1]); <-- Everything else garbage
printf ("2:%s\n", inx, value[2]);
printf ("3:%s\n", inx, value[3]);
printf ("4:%s\n", inx, value[4]);

memset (str, NULL, 1000);
fgets( str, sizeof(str), fp );
}
}

fclose(fp);
return ret;
}

wrkspace *parse_record(char (*record) [], int fid, int structsz) {
int i=0;
int j;
char *token;
char sep_1[] = "|";
char sep_2[] = "'";
wrkspace value;

token = strtok( *record, sep_1 );

while( token != NULL ) { // While there are tokens in "string"
memset (value, NULL, 200);
strcpy (value, token);
token = strtok( NULL, sep_1 ); // Get next token:

i++;
}

j = i - 2; // Minus 2 is remove the last '|'

for (i=0; i <= j; i++) {
token = strtok (value, sep_2);
strcpy (value , token);
}
return &value;
}

 

[Barry Schwarz]

Relatively new to C coding, so any help would greatly be appreciated.
I'm having problems try to return my string array from my parsing
function. When I do a printf I am getting the correct value for my first
element but my subsequent printf's will return garbage. Can someone let
me know what I am doing wrong. Thanks in advance.

-jlewis



typedef char wrkspace[40][200]; <- Locatated in my haeder


int open_file (char *path, char *recNo, int fid) {
FILE *fp;
char str [1000];
int ret;
int count;
int inx;
int i;
wrkspace *value;

if ((fp=fopen(path, "r"))==NULL) {
printf("Cannot open file.\n");
return -1;
}

count = atoi(recNo);
memset (str, NULL, 1000);
fgets( str, sizeof(str), fp );

while (!feof(fp)) {
for (inx = 0; (inx < count ) && (!feof(fp)) && ( strlen(str) > 0

Why do you test feof(fp) twice, once in the while and once in the for?
Neither is correct. feof does not tell you there is no more data; it
tells you the last read attempted to go beyond the end of the data.

); inx++) {
<--CALLING ROUTINE --->
value = (wrkspace *) parse_record(&str, fid, count);

There is no prototype in scope for parse_record. A C89 compiler is
required to assume the function returns an int. Since it does not,
you have invoked undefined behavior. Create a prototype. Once you
have that, you can eliminate the superfluous cast.

printf ("0:%s\n", inx, value[0]); <-- returns good data

Only by accident. value is a pointer to an array of 40 arrays of 200
char. value[0] is the first array of 40 arrays of 200 char at that
address. When an array is passed to a function, the compiler actually
passes the address of the first element with type pointer to element.
Thus, the address passed is the address of the first array of 200
char. Unfortunately, it also has type pointer to array of char which
is the wrong type for the %s format. This also invokes undefined
behavior.

printf ("1:%s\n", inx, value[1]); <-- Everything else garbage

value[1] is the second array of 40 arrays of 200 char. This address
is some 800 bytes beyond value[0]. Not at all what you intended.

printf ("2:%s\n", inx, value[2]);
printf ("3:%s\n", inx, value[3]);
printf ("4:%s\n", inx, value[4]);

memset (str, NULL, 1000);

The second argument has type int. On many systems, NULL is defined as
(void*)0 which is incompatible with int. When you mean zero, use 0.

fgets( str, sizeof(str), fp );
}
}

fclose(fp);
return ret;

ret is never initialized. Attempting to evaluate an uninitialized
variable invokes undefined behavior.

}

wrkspace *parse_record(char (*record) [], int fid, int structsz) {
int i=0;
int j;
char *token;
char sep_1[] = "|";
char sep_2[] = "'";
wrkspace value;

value is an automatic variable that exists only for the duration of
this function.

token = strtok( *record, sep_1 );

while( token != NULL ) { // While there are tokens in "string"
memset (value, NULL, 200);

This doesn't serve any purpose. The following strcpy does not require
the target area to be initialized.

strcpy (value, token);
token = strtok( NULL, sep_1 ); // Get next token:

i++;
}

j = i - 2; // Minus 2 is remove the last '|'

for (i=0; i <= j; i++) {
token = strtok (value, sep_2);
strcpy (value , token);

token points somewhere in value. Since the arguments to strcpy
overlap, you have invoked undefined behavior. memmove may do what you
want.

}
return &value;

You are returning the address of a variable that will disappear as
soon as the function exits. Any attempt to access the variable, as
you do in open_file, invokes undefined behavior.

}

You can solve many of these problems by:

Change the typedef for workspc to char[200]

Change the first parameter of parse_record to char* and specify
str (not &str) in the function call. This will require you to change
*record to record in the body.

In parse_record, change value to a pointer and initialize it with
the address of an allocated area obtained from malloc:
value = malloc(40 * sizeof *value);


<<Remove the del for email>>