Question

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?

Was it helpful?

Solution

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.

OTHER TIPS

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?

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);

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?)

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.

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.

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.

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top