11

In an earlier question, I asked about typecasting pointers, but was directed to the better solution of using the C++ allocation system instead of mallocs. (I am converting some C code to C++)

However, I still have an issue with a similar function:

I changed:

tmp = malloc(sizeof(char*) * mtmp); --> tmp = new char*[mtmp];

and

free(tmp) --> delete [] tmp;

However, what do I do with realloc in the following function:

char* space_getRndPlanet (void)
{
   int i,j;
   char **tmp;
   int ntmp;
   int mtmp;
   char *res;

   ntmp = 0;
   mtmp = CHUNK_SIZE;
   //tmp = malloc(sizeof(char*) * mtmp); <-- replaced with line below
   tmp = new char*[mtmp];
   for (i=0; i<systems_nstack; i++)
      for (j=0; j<systems_stack[i].nplanets; j++) {
         if(systems_stack[i].planets[j]->real == ASSET_REAL) {
            ntmp++;
            if (ntmp > mtmp) { /* need more space */
               mtmp += CHUNK_SIZE;
               tmp = realloc(tmp, sizeof(char*) * mtmp); <--- Realloc
            }
            tmp[ntmp-1] = systems_stack[i].planets[j]->name;

I am getting the following error:

error: invalid conversion from 'void*' to 'char**'|

EDIT 2:

Okay, the consensus I am getting is that I should ditch my current solution (which I am open to doing).

Just to make sure that I am understanding correctly, do you guys mean that, instead of an array of pointers to objects, I should just have a vector containing the objects themselves?

Biosci3c
  • 772
  • 5
  • 15
  • 35
  • 1
    Well ignoring the fact that you are using realloc() incorrectly in the above piece of code. This is a weakness of C++ caused because objects have constructors/destructors. The problem is resolved by using vector rather than raw arrays. – Martin York Oct 06 '10 at 03:02

8 Answers8

15

C allows void* to be implicitly converted to any pointer. C++ doesn't, so if you're using realloc, you have to cast the result to the appropriate type.

But more importantly, using realloc on a pointer returned by new[] is undefined behavior. And there's no direct C++-style equivalent to realloc.

Your choices are, from least to most idiomatic:

  • Stick to malloc/realloc/free and cast the pointers.
  • Use new[] + delete[] instead of realloc
  • Use std::vector<std::string> instead of managing your own memory.
KitsuneYMG
  • 12,753
  • 4
  • 37
  • 58
dan04
  • 87,747
  • 23
  • 163
  • 198
14

This appears to be an unremarkable array that grows as needed.

Stop using explicit memory allocation, you almost certainly have no need of it. Use std::vector or another of the C++ standard library's dynamic containers that automatically grow as needed for you.

It also looks like you're using null-terminated C-style strings. Why not used std::string instead?

Nicholas Knight
  • 15,774
  • 5
  • 45
  • 57
  • I'm converting some old C code, and learning as I go (not familiar with C much, just C++). So this might work. – Biosci3c Oct 06 '10 at 03:00
5

In C++ you should not be using arrays (even dynamically allocated).

This has been replaced with std::vector

In C:

char** tmp = (char**)malloc(sizeof(char*) * size);
free(tmp);

// And a correct version of realloc
char** alt = (char**)realloc(sizeof(char*) * newSize);
if (alt)
{
    // Note if realloc() fails then it returns NULL
    // But that does not mean the original object is deallocated.
    tmp = alt;
}

In C++

std::vector<char*>   tmp(size);

// No need for free (destructor does that).
tmp.resize(newSize);
Martin York
  • 257,169
  • 86
  • 333
  • 562
2

http://www.cplusplus.com/forum/general/18311/

(In short - there's not really a C++ equivalent to realloc, but maybe you'd be better of using a vector?)

Amber
  • 507,862
  • 82
  • 626
  • 550
  • Could you explain to me what exactly realloc is doing here? Also, by replacing the current solution with vectors does this mean that, instead of a stack, i am using an array-like device (i.e. vector). I.e.: does this mean I should replace the current stack solution? – Biosci3c Oct 06 '10 at 02:58
  • @Biosci3c You can also use a stack then. C++ defines both types in the standard template library. See http://www.cplusplus.com/reference/stl/ – zneak Oct 06 '10 at 03:02
  • 1
    @Biosci3c - realloc is creating a bigger memory space to hold the contents of what's in `tmp`, because the old space was filled up. Re-sizable C++ containers do this automatically for you. You can use a `stack` or a `vector`; both of them support "stack-like" operations (but the `vector` also supports random access, whereas the `stack` does not). – Amber Oct 06 '10 at 03:05
  • Also, in your current solution, `tmp` is not a stack, it is an array. – Amber Oct 06 '10 at 03:07
  • 1
    Realloc will try to resize the currently allocated memory block. If it can't (say, you're trying to enlarge it and there's another block in the way), it will allocate a new memory block, copy the pointer's content and return a new pointer to the structure. (The original pointer's not changed unless you do so). Given the nature of objects in C++ (deep copy vs shallow copy, for one), this kind of logic must be handled by the user since the compiler can't know how things need to be copied. – Mauricio Oct 06 '10 at 03:16
2

Realloc doesnt reallly care about calling constructors, so using realloc after new seems like a bad idea.You should be going for vector as a better approach. You can resize vectors.

Alok Save
  • 202,538
  • 53
  • 430
  • 533
1

Unfortunately there's no realloc in C++ (due to the fact that memory might hold non-trivially constructed or just non-copyable objects). Either allocate new buffer and copy or learn to use std::vector or std::stack that would do this for you automatically.

Nikolai Fetissov
  • 82,306
  • 11
  • 110
  • 171
1

I'd consider switching from the hand-crafted dynamic array to a vector a good first choice.

As far as whether the vector should contain objects directly, or contain pointers to the actual objects, there isn't a single definitive answer -- it depends on a number of factors.

The single biggest factor is whether these are what I'd call "entity" objects -- ones for which copying makes no sense. A classic case in point would be something like an iostream or a network connection. There's generally no logical way to copy or assign an object like this. If that's the sort of object you're dealing with, you're pretty much stuck storing pointers.

If, however, you're dealing with what are generally (loosely) defined as "value objects", copying and assigning will be fine, and storing the objects directly in the vector will be fine. With objects like this, the primary reason to store pointers instead would be that a vector can/will copy objects around when it has to expand the memory to make room for more objects. If your objects are so large and expensive to copy that this is unacceptable, then you might want to store something pointer-like in the vector to get cheap copying and assignment. Chances are that will not be a raw pointer though -- it'll probably be some sort of smart pointer object that provide more value-like semantics so most other code can treat the object as being a simple value, and the details of its expensive operations and such can remain hidden.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
-1

There is no C++ equivalent of realloc().

Best use:

char* new_tmp = new (char*)[mtmp];
for (int n=0;n<min(mtmp,oldSize);n++) new_tmp[n] = tmp[n];
delete [] tmp;  // free up tmp
tmp = new_tmp;

The error you are getting is because C++ is less lenient with implicit type casting.

Alexander Rafferty
  • 6,134
  • 4
  • 33
  • 55
  • 1
    Not the best use - the idea of `realloc` is to retain memory content. – Nikolai Fetissov Oct 06 '10 at 02:59
  • 1
    If you insist on doing manual memory allocation, you should allocate the new memory, then copy the contents to the newly allocated memory. Only after both those succeed should you delete the old memory. – Jerry Coffin Oct 06 '10 at 03:32