views:

723

answers:

5

Hello All,

I am learning C++. I am trying to learn this dynamic memory allocation. In the below code I am trying to allocate memory using malloc and realloc.

int main (void)  {
  char *g = (char*) malloc (sizeof(char) * 2);
  g = "ab";
  g = (char*) realloc (g, sizeof(char) * 200);
  strcpy (g, "cdefg");
  cout << g << endl;
  return 0;
}

I get segmentation fault error when I execute this code.

I've seen other SO threads recommending to use either vector or new instead of using above approach. Since, I am trying to learn this approach, so these answers does not applies to my question.

I've encountered a scenario where this approach will be well fitted, for example, If I am reading a raw text file using ifstream.read function and reading, let's say 1024bytes of it. Now, If I want to make sure I am not reading a word which is broken due to 1024bytes size I have to read further from that position using ifstream.get to reach until a space character is found. In this scenario, I need to adjust the buffer (which is of 1024) slightly more. My intention is to use realloc here to assign some more memory to it.

Please feel free to correct me if I am wrong anywhere.

+9  A: 

In

g = "ab";

you make g point to a chunk located in static storage, not on heap, then in

g = (char*) realloc (g, sizeof(char) * 200);

you try to call realloc() with that address belonging outside the heap. This is undefined behaviour and crashes your program. You can call realloc() only with addresses returned by malloc() or realloc() (or null pointer).

The problem is that you meant this:

strcpy( g, "ab" );

but mistakenly wrote this:

g = "ab";

The former would work allright - g is initially set to an address returned by malloc().

There's one more problem - you allocate too little memory initially. You should allocate an extra byte for the null terminator.

sharptooth
+3  A: 

The problem is that the line:

g = "ab";

sets the g pointer to another location altogether. It does not copy the contents. You lose track of the original location and when you use g in realloc, you'll be reallocing the wrong pointer.

Replace the line with:

strcpy(g, "ab");

Of course, you'll need three bytes to store the string and the terminating '\0' character.

That said, malloc and realloc are C style things. If you are using C++, you should try new and delete and probably use string type to store strings.

Mehrdad Afshari
+4  A: 

There are many problems with your code. First of all, you should not use malloc and friends if you are programming in C++.

char *g = (char*) malloc (sizeof(char) * 2);
g = "ab";

Ooops. You just lost the 2 bytes of memory returned by the malloc call because now g points to the possibly read only location where "ab" is stored.

g = (char*) realloc (g, sizeof(char) * 200);

realloc can only be called on a pointer returned by an earlier malloc.

Even if you had passed a valid pointer to realloc, realloc may return NULL if the re-allocation fails. In that case, the memory previously allocated remains allocated but you would overwrite the only variable pointing to that memory making it impossible to free the earlier allocation. See Having dynamically allocated an array, can I change its size? in the C FAQ list.

Also, note that "ab" requires three bytes of storage, not two. Finally, sizeof(char) is always and everywhere 1, so there is no need to use sizeof(char) in your malloc calls.

A correct C version of your program would look like this:

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

int main (void)  {
    char *tmp;
    char *g = malloc(3);
    if ( !g ) {
        return EXIT_FAILURE;
    }

    strcpy(g, "ab");

    tmp = realloc (g, 200);
    if ( !tmp ) {
        return EXIT_FAILURE;
    }
    g = tmp;

    strcpy (g, "cdefg");
    puts(g);
    return 0;
}

In the C++ version, you would use string rather than plan old C style character arrays.

See my answer to another question for an example of how to reallocate a buffer to read complete lines for the second part of your question.

However, note that code is also C and I am sure there are better ways of doing what you want in C++. I just do not know enough of the C++ standard library to give you a correct solution in C++.

Sinan Ünür
Can explain me the solution for your last para? Why realloc can't be used if it is safely handled.
learner
@learner Link to C FAQ added.
Sinan Ünür
If the code above is to be called correct, it should check the return value of `malloc` and ensure is not `NULL`.
Idelic
Thanks Sinan, It was a great help!!
learner
This is a *really* nice answer
Draemon
+3  A: 

You need to increase the original malloc to 3 to include the null terminator, and use strcpy to set the contents to "ab"

char *g = (char*) malloc (sizeof(char) * 3);
strcpy(g , "ab");
David Sykes
Yep. Nice catch.
Amit
A: 

Re the second part of your question:

Consider using std::vector.

std::vector<char> g;
g.resize(1024);
// fill up g
// decide you need another hundred bytes at the end of g
g.resize(1124);
// memory will automatically be released when g goes out of scope

Note that to pass a pointer to the start of the buffer, you will now have to say &g[0] instead of simply g.

Dave Hinton
the problem is, if i've to read data from a file ifstream.read takes only char* not std::string. So, you cannot skip the above approach... Is it right?
learner
Dave Hinton