0

I'm working on a little C program for a class that reads the lines in from a file and then sorts them using qsort. Long story short, I am dynamically allocating memory for every line of a file, stored as a char*, in an array of char*. The reading in and storing ostensibly works fine based upon the output (see below), but when I print out the lines, they are all duplicates of the last line in the file. Can anyone point out my (most likely painfully obvious) error?

Here is the relevant code to the problem I'm currently running into:

char* trim_white_space(char* str);
char* get_line(FILE* infile, char temp[]);

int main(int argc, char* argv[]) {
    FILE* infile; 
    char* input_file = argv[1];
    int cnt = 0;
    char temp[MAX_LINE_LENGTH]; //to hold each line as it gets read in 
    char* tempPointer = temp;

    if (argc < 2) {
        printf("No input file provided");
        return EXIT_FAILURE;
    }

    //determine the number of lines in the file
    infile = fopen(input_file, "r");
    int num_lines_in_file = num_lines(infile);
    fclose(infile);

    //allocate pointers for each line
    char** lines = (char**) malloc(num_lines_in_file * sizeof(char*));

    //temporarily store each line, and then dynamically allocate exact memory for them
    infile = fopen(input_file, "r");
    for (cnt = 0; cnt != num_lines_in_file; cnt++) {
        tempPointer = get_line(infile, temp);  
        lines[cnt] = (char*) malloc(strlen(tempPointer) + 1);
        lines[cnt] = trim_white_space(tempPointer);
        printf("%d: %s\n", cnt, lines[cnt]);
    }

    fclose(infile);

    //print the unsorted lines (for debugging purposes)
    printf("Unsorted list:\n");
    for (cnt = 0; cnt != num_lines_in_file; cnt++) {
        printf("%s\n", lines[cnt]);
    }

char* get_line(FILE* infile, char temp[]) {
    fgets(temp, MAX_LINE_LENGTH-1, infile);
    char* pntr = temp;
    return pntr;
}

char *trimwhitespace(char *str)
{
  char *end;

  // Trim leading space
  while(isspace(*str)) str++;

  if(*str == 0)  // All spaces?
    return str;

  // Trim trailing space
  end = str + strlen(str) - 1;
  while(end > str && isspace(*end)) end--;

  // Write new null terminator
  *(end+1) = 0;

  return str;
}

I have this sample input file 5-1input.dat:

Hi guys
x2 My name is
Slim Shady
For real

And here's the output I'm getting:

user@user-VirtualBox ~/Desktop/Low-level/HW5 $ ./homework5-1 5-1input.dat 
0: Hi guys
1: x2 My name is
2: Slim Shady
3: For real
Unsorted list:
For real
For real
For real
For real
3
  • 2
    Please add the code of trim_white_space. In short all lines entries are set to point to the string returned by this function, which likely is tempPointer itself or some static global. In any case it overwrites the pointer returned by malloc. Commented Aug 17, 2013 at 22:45
  • 1
    True that. You firstly assign the result of malloc, then you assign the result of trim_white_space, so malloc is lost. Try strncpy. Commented Aug 17, 2013 at 22:49
  • I've added it. I got that code from another stackoverflow post. Commented Aug 17, 2013 at 22:54

1 Answer 1

1

As in the comments, you should change your loop to:

for (cnt = 0; cnt != num_lines_in_file; cnt++) {
    tempPointer = get_line(infile, temp);  
    lines[cnt] = (char*) malloc(strlen(tempPointer) + 1);
    strncpy(lines[cnt], trim_white_space(tempPointer), strlen(tempPointer)+1);
    printf("%d: %s\n", cnt, lines[cnt]);
}

The size in strncpy is based on the size of malloc you've used.

Of course you can optimize this code, e.g. to count strlen only once, etc.

Sign up to request clarification or add additional context in comments.

4 Comments

Wow, thank you! Can you explain why I cannot allocate the memory in one assignment and then make that point to the trimmed string with another assignment?
@Hooked: Simply put, how do you know the allocation succeeded? malloc returns NULL on failure, which means you can't use the pointer it returns. You are checking to ensure that any calls to malloc and functions making use of malloc, realloc, etc. don't return NULL, right?
trim_white_space returns a pointer. It's really just a number, say 0x1234567. Under this address you have your data, which you can access with * operator. lines[cnt] = (char*) malloc(strlen(tempPointer) + 1); means - allocate some memory for my data, give me it's address and keep it in lines[cnt]. lines[cnt] = trim_white_space(tempPointer); means - execute a function on tempPointer, take the address it returns and overwrite data (the pointer to allocated memory) already stored in lines[cnt]. What you want to do is to rewrite the content from the memory under one pointer to another.
That's what strncpy is for. You may want to take a look at strndup as well, it does the malloc for you. Of course remember to call free after you're done with your data. Also, everything in Chrono Kitsune comment is important as well - memory allocation may fail, so you have to check if it succeeded, or prepare for serious problems ;-) P.S. Sorry for lousy formatting in my previous comment, I can't edit it anymore...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.