How to convert my malloc + strcpy to strdup in C?

Question

I am trying to save csv data in an array for use in other functions. I understand that strdup is good for this, but am unsure how to make it work for my situation. Any help is appreciated!

The data is stored in a struct:

typedef struct current{
    char **data;
}CurrentData;

Function call:

int main(void){
    int totalProducts = 0;
    CurrentData *AllCurrentData = { '\0' };
    FILE *current = fopen("C:\\User\\myfile.csv", "r");

    if (current == NULL){
        puts("current file data not found");
    }
    else{
        totalProducts = getCurrentData(current, &AllCurrentData);
    }
 fclose(current);
 return 0;
 }

How I allocated memory;

 int getCurrentData(FILE *current, CurrentData **AllCurrentData){

*AllCurrentData = malloc(totalProducts * sizeof(CurrentData));

    /*allocate struct data memory*/
    while ((next = fgetc(current)) != EOF){                                                 
        if (next == '\n'){
            (*AllCurrentData)[newLineCount].data = malloc(colCount * sizeof(char*));
            newLineCount++;
        }
    }
    newLineCount = 0;
    rewind(current);

    while ((next = fgetc(current)) != EOF && newLineCount <= totalProducts){                    

        if (ch != '\0'){
            buffer[i] = ch;
            i++;
            characterCount++;
        }

        if (ch == ',' && next != ' ' || ch == '\n' && ch != EOF){
            if (i > 0){
                buffer[i - 1] = '\0';
            }
            length = strlen(buffer);
            /*(*AllCurrentData)[newLineCount].data[tabCount] = malloc(length +  1);     /* originally was using strcpy */
            strcpy((*AllCurrentData)[newLineCount].data[tabCount], buffer);
            */
            (*AllCurrentData)[newLineCount].data[tabCount] = strdup(buffer);  /* something like this? */

            i = 0;
            tabCount++;

            for (j = 0; j < BUFFER_SIZE; j++){
                buffer[j] = '\0';
            }
        }

This is not the complete code. You have several variables which are not declared or set. There's no way to tell exactly what the code is doing from these fragments. Also it's not clear what the CurrentData type is supposed to contain. — Chris J. Kiick, Jan 04 '14 at 18:25
It looks like what you are wanting to do is to open a file containing lines of text, in this case comma separated values text, and read each line into an array of string buffers where each array element points to an allocated memory area containing a text string. The result should be an array of pointers to strings where the order of the of the strings is the same as the order of the lines in the file. So array[0] should point to a buffer containing the text of the first line of text in the file. Is that what you are wanting to do? — Richard Chambers, Jan 04 '14 at 18:28
At first glance, `strdup` is not your problem; your real problem is exhaustive allocation. For example you pass the file twice and count the newlines in the first pass. That's fine, but why do you allocate before that pass when you don't know how much to allocate? — M Oehm, Jan 04 '14 at 18:29

score 0 · Answer 1 · answered Jan 04 '14 at 18:35

Okay, I wouldn't comment on other parts of your code, but you can use strdup to get rid of this line (*AllCurrentData)[newLineCount].data = malloc(colCount * sizeof(char*));, and this line (*AllCurrentData)[newLineCount].data[tabCount] = strdup(buffer); /* something like this? */ and replace them with this: (*AllCurrentData)[newLineCount].data = strdup(buffer);

AndersK · Accepted Answer · 2014-01-05T23:03:03.580

You define a ptr AllCurrentData but you should set it to NULL.

CurrentData* AllCurrentData = NULL;

In getCurrentData you use totalProducts which seems a bit odd since it is a local variable in main(), either you have another global variable with the same name or there is an error.

The **data inside the structure seems odd, instead maybe you want to parse the csv line and create proper members for them. You already have an array of CurrentData so it seems odd to have another array inside the struct -- i am just guessing cause you haven't explained that part.

Since a csv file is line based use fgets() to read one line from the file, then parse the string by using e.g. strtok or just by checking the buffer after delimiters. Here strdup can come into play, when you have taken out a token, do a strdup on it and store it in your structure.

char line[255];
if ( fgets(line,sizeof(line),current) != NULL )
{
  char* token = strdup(strtok( line, "," ));
  ...
}

Instead of allocating a big buffer that may be enough (or not) use realloc to increase your buffer as you read from the file.

That said there are faster ways to extract data from a csv-file e.g. you can read in the whole file with fread, then look for delimiters and set these to \0 and create an array of char pointers into the buffer.

Thanks for the tip on csv data extraction, I think I will rewrite the program using the method you suggested. — TinMan, Jan 04 '14 at 19:54

Richard Chambers · Answer 3 · 2014-01-04T19:07:29.083

For the function to read in the array of strings I would start with the following approach. This has not been tested or even compiled however it is a starting place.

There are a number of issues not addressed by this sample. The temporary buffer size of 4K characters may or may not be sufficiently large for all lines in the file. There may be more lines of text in the file than elements in the array of pointers and there is no indication from the function that this has happened.

Improvements to this would be better error handling. Also it might be modified so that the array of pointers is allocated in the function with some large amount and then if there are more lines in the file than array elements, using the realloc() function to enlarge the array of pointers by some size. Perhaps also a check on the file size and using an average text line length would be appropriate to provide an initial size for the array of pointers.

// Read lines of text from a text file returning the number of lines.
// The caller will provide an array of char pointers which will be used
// to return the list of lines of text from the file.
int GetTextLines (FILE *hFile, char **pStringArrays, int nArrayLength)
{
    int  iBuffSize = 4096;
    int  iLineCount = 0;
    char tempBuffer [4096];

    while (fgets (tempBuffer, iBuffSize, hFile) && iLineCount < nArrayLength) {
        pStringArrays[iLineCount] = malloc ((strlen(tempBuffer) + 1) * sizeof (char));
        if (! pStringArrays[iLineCount])
            break;
        strcpy (pStringArrays[iLineCount], tempBuffer);
        iLineCount++; 
    }
    return iLineCount;
}

How to convert my malloc + strcpy to strdup in C?

3 Answers3