how to malloc memory for string pointers array, and other problems

Question

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_LINE_SIZE 100
#define INITIAL_BUFFER_SIZE 16

int main() {

    char **line_buffer;
    line_buffer = (char **) malloc(INITIAL_BUFFER_SIZE);
    int i = 0;
    int lines = 0;
    // the size of buffer
    int buffer_size = 1;
    int *buffer_size_p = &buffer_size;
    char *line_one = (char *) malloc(MAX_LINE_SIZE);
    //read lines from a file
    while (gets(line_one)) {
        line_buffer[lines] = (char *) malloc(MAX_LINE_SIZE);
        strcpy(line_buffer[lines], line_one);
        lines++;
        line_one = (char *) malloc(MAX_LINE_SIZE);
            // if too much lines double the buffer size
        if (lines == *buffer_size_p) {
            buffer_size *= 2;
            line_buffer = IncreaseBuffer(line_buffer, buffer_size_p);
        }
    }
    PrintLines(line_buffer, lines);
    // sorting all the line by strcmp
    for (i = 0; i < lines; i++) {
        printf("%s", line_buffer[i]);
//        int min = MinLineIndex(line_buffer, i, lines);
//        SwapLines(line_buffer, i, min);
    }


    PrintLines(line_buffer, lines);
//    free(line_buffer);
    return 0;
}

First, ignore the gets() function, this is required for this.

First, I used a for loop to do (char *) line_buffer[lines] = (char *) malloc(MAX_LINE_SIZE);

It does not work;

I did this way, it worked, However, after read several lines from a file, the first line become something like "��R", and it is changing every time.

And, I cannot use free(line_buffer); as well.

Self studying. Please help.

What is `INITIAL_BUFFER_SIZE`? What is `MAX_LINE_SIZE`? You also have much more allocations than you need, and very bad memory leaks in the loop. You should also never use `gets`, it has been obsolete since the C99 standard and removed completely from the latest C11 standard. Lastly, [in C you should not cast the return of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) (or other functions returning `void *`). — Some programmer dude, Feb 14 '16 at 18:15
First, ignore the gets() function, this is required for this. — , Feb 14 '16 at 18:19
after read several lines from a file, the first line become something like "��R", and it is changing every time. — , Feb 14 '16 at 18:20
"after several lines" ... after *one* line you invoke `IncreaseBuffer`, the guts of which we haven't a clue. Post *compilable* code, and include a sample data set that reproduces your problem. — WhozCraig, Feb 14 '16 at 18:20
We also don't know if there's anything wrong in the `IncreaseBuffer`, how do we know it calls `realloc` (I assume) correctly? — Some programmer dude, Feb 14 '16 at 18:28
@JoachimPileborg We don't know. Even as-is the code stands a chance of working so long as (a) the size of a `char*` on the OP's platform is no more than 16 octets (likely), and (b) `IncreaseBuffer` *works*. I'm strongly hedging bets the latter is not the case. — WhozCraig, Feb 14 '16 at 18:32
I have rolled this question back to the last state the question was in before it was changed to the answer. If you wish to provide an answer, please do so below - changing the question into an answer makes it an answer without a question, which is confusing for new readers. — halfer, Mar 16 '16 at 23:58

Some programmer dude · Accepted Answer · 2016-02-14T18:27:14.177

A big problem is that you only allocate 16 bytes for line_buffer, which on a 64-bit system would mean you only allocate space for two pointers.

If you read more than two (or four on 32-bit systems) lines you will go out of bounds.

If you want to allocate space for 16 pointers, then you need to allocate space for INITIAL_BUFFER_SIZE * sizeof(char *) bytes, or better yet, INITIAL_BUFFER_SIZE * sizeof(*line_buffer).

Then there's the issue of memory leaks, of which you have quite a few. First of all you don't need to allocate memory for the temporary buffer line_one, declare it as a normal fixed-size array. That will get rid of quite a few leaks as you reallocate it in the loop without freeing the old memory.

Then to free the line_buffer memory you actually need to free each individual entry first, before you call free on line_buffer.

Remember: For each malloc you need a corresponding free.

And instead of allocating and copying each line explicitly, while it's not a standard C function just about all libraries have a strdup function which does it for you, so you can do e.g.

line_buffer[lines] = strdup(line_one);

And the buffer_size_p variable is not needed, if you need to use a pointer to the buffer_size variable, just use the address-of operator directly when needed, like in

line_buffer = IncreaseBuffer(line_buffer, &buffer_size);

This is one reason you shouldn't hard code byte sizes into `malloc` calls. — e0k, Feb 14 '16 at 18:20
malloc(INITIAL_BUFFER_SIZE * sizeof(char *)) Because of this, i did a lot stupid work to fix........ Thank you so much — , Feb 14 '16 at 18:31

score 0 · Answer 2 · answered Feb 14 '16 at 18:23

0

From what you've posted your use (or, rather incorrect) use malloc may be the culprit.

For line buffer, you are only allocating 16 bytes, when really you should have been doing this:

line_buffer = (char **) malloc(INITIAL_BUFFER_SIZE*sizeof(char*));

Also, I would also encourage the use of sizeof in your other malloc call:

line_buffer[lines] = (char *) malloc(MAX_LINE_SIZE*sizeof(char))

answered Feb 14 '16 at 18:23

shafeen

2,431
18
23

`sizeof(char)` is defined in the C specification to always be `1`. – Some programmer dude Feb 14 '16 at 18:29
yes, but it's just a good habit to have, it definitely does not hurt – shafeen Feb 14 '16 at 18:29

score 0 · Answer 3 · answered Feb 14 '16 at 18:25

0

The next crucial thing is:

int buffer_size = 1;

should be:

int buffer_size = INITIAL_BUFFER_SIZE;

After, all, you just allocated INITIAL_BUFFER_SIZE pointers to lines.

answered Feb 14 '16 at 18:25

Paul Ogilvie

25,048
4
23
41

"... you just allocated INITIAL_BUFFER_SIZE pointers to lines. " - No, he didn't he allocated `INITIAL_BUFFER_SIZE` *bytes*. – WhozCraig Feb 14 '16 at 18:25
Yes, and that comment is already made, so that's why I say "the _next_ crucial thing" – Paul Ogilvie Feb 14 '16 at 18:27

score -1 · Answer 4 · answered Mar 16 '16 at 23:59

(Posted on behalf of the OP).

First, ignore the gets() function, this is required for this:

 (char **) malloc(INITIAL_BUFFER_SIZE * sizeof(char *))

Almost all the problems are because I don't know I need to use * sizeof(char *).

Here is my final code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_LINE_SIZE 100
#define INITIAL_BUFFER_SIZE 16

int main() {

    char **line_buffer;
    line_buffer = (char **) malloc(INITIAL_BUFFER_SIZE * sizeof(char *));
    int i = 0;
    int lines = 0;
    for (i = 0; i<INITIAL_BUFFER_SIZE; i++){
        line_buffer[i] = (char *) malloc(MAX_LINE_SIZE * sizeof(char));
    }
    // the size of buffer
    int buffer_size = INITIAL_BUFFER_SIZE;
    while (gets(line_buffer[lines++])) {
        if (lines == buffer_size) {
            buffer_size *= 2;
            line_buffer = IncreaseBuffer(line_buffer, &buffer_size);
        }
    }
    // sorting all the line by strcmp
   for (i = 0; i < lines; i++) {
       int min = MinLineIndex(line_buffer, i, lines - 2);
       SwapLines(line_buffer, i, min);
   }


    PrintLines(line_buffer, lines - 1);
    for (i = 0; i < buffer_size ; i++) {
        free(line_buffer[i]);
    }
    free(line_buffer);
    return 0;
}

how to malloc memory for string pointers array, and other problems

4 Answers4