views:

212

answers:

8

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?

+2  A: 

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
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
@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
@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
Also, in your current solution, `tmp` is not a stack, it is an array.
Amber
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.
Kyte
A: 

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
Not the best use - the idea of `realloc` is to retain memory content.
Nikolai N Fetissov
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
A: 

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 N Fetissov
+9  A: 

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
I'm converting some old C code, and learning as I go (not familiar with C much, just C++). So this might work.
Biosci3c
+5  A: 

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.
dan04
Plus you shouldn't be mixing new/delete and malloc/calloc/realloc/free memory allocation.
Paul McGuire
+1  A: 

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.

Als
+4  A: 

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
+1  A: 

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