Trying to delete void* ?!

Good afternoon. How can I proceed to free the memory allocated to tmp?

Code:

/* 
 * File:   memswap.cpp
 * Author: thames
 *
 * Created on 7 de Janeiro de 2013, 14:46
 */

#include <cstdlib>
#include <iostream>
#include <cstring>

int swap(void* x, void* y, size_t size)
{
   using std::memcpy;
    
   decltype(x) tmp;
   if( (tmp = new size_t) == nullptr)
    return -1;   
   memcpy(tmp, x, size);
   memcpy(x, y, size);
   memcpy(y, tmp, size);
   // delete tmp;
   
   return 0;
}

int main() {

    using std::cout;
    using std::endl;
    
    int x = 10; 
    int y = 20;
    size_t size = sizeof(int);
    if(swap(&x, &y, size) == 0)
    { 
      cout << x << " " << y << endl; 
      return EXIT_SUCCESS;
    }     
    else 
    {  
      cout << "An error has occurred.\n";    
      return EXIT_FAILURE;
    }
}

What is the type of tml? dcltype? This does not even compile on my machine. Why not using the new keyword?

1. new size_t allocates a block of exactly sizeof(size_t) bytes which is probably 4 or 8.
2. You cannot delete void*. The only way to free the memory you are currently allocating is to cast the pointer to size_t* and then delete it.
3. decltype(x) is a compile-time operator, which always returns void* in your case. "tmp" is always of type void*.
4. If you want a universal swap for types other than trivially copyable (PODs), you must use templates. Note: std::swap already exists if you don't know.
5. If you want to allocate and free a raw storage, use char* tmp = new char[size], then delete[] tmp.

In other words, you should change your approach.

Note: std::swap already exists if you don't know.

I already knew that. I'm starting to read a book called Mastering Algorithms with C which teaches Data Structures with C and there was a swap method in C using memcpy and I was trying to translate it to C++ . That cast made the trick!

Code:

 
delete (size_t*) tmp;

decltype as kmdv pointed out, is a operator for deciding the type of the variable in compile time. (C++11 usage only).

Yeah I read his post.. Well, the portability should always be on top

Yeah I read his post.. Well, the portability should always be on top http://im.cprogramming.com/images/smilies/wink.png

haha

Originally Posted by thames

I already knew that. I'm starting to read a book called Mastering Algorithms with C which teaches Data Structures with C and there was a swap method in C using memcpy and I was trying to translate it to C++ .

To translate it to C++ you use the delete key on your keyboard and use the functionality the language provides.

This code is horribly broken in so many ways that you cant imagine yet. I can think of seven major issues right off the bat:
1. It allows non-copyable, non-swappable types to be swapped. Disaster for those objects, and nothing is detected at compile-time.
2. It uses return values to indicate success or failure instead of exceptions.
3. It assumes that a bitwise copy of objects will work, which is not true for certain classes such as those that contain pointers to their own members.
4. A bitwise swap can be done without allocating any additional memory.
5. The types are unknown here, so the appropriate member functions such as assignment operators cannot be called.
6. The size must be passed in, which should not be required.
7. The function should take references instead of pointers so that it doesn't have to deal with NULL pointers, which by the way you're failing to acount for.

Your last post shows that you're calling the wrong version of delete too now.
There is no way in hell this code should ever see the light of day. Attempting to use it in a real C++ program would be catastrophic.

Originally Posted by thames

I already knew that. I'm starting to read a book called Mastering Algorithms with C which teaches Data Structures with C and there was a swap method in C using memcpy and I was trying to translate it to C++ ...

Why are you trying to do such a thing? C is not C++!

ok

the original C code:

Code:

/* 
 * File:   memswap.c
 * Author: thames
 *
 * Created on 7 de Janeiro de 2013, 12:55
 */

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

int swap(void* x, void* y, size_t size)
{
   void* tmp;
   if( (tmp = malloc(size)) == NULL)
    return -1;   
   memcpy(tmp, x, size);
   memcpy(x, y, size);
   memcpy(y, tmp, size);
   free(tmp);
   
   return 0;
}

int main(int argc, char** argv) {

    int x = 10; 
    int y = 20;
    size_t size = sizeof(int);
    if(swap(&x, &y, size) == 0)
    { 
      printf("%d %d\n", x, y);
      return EXIT_SUCCESS;
    }     
    else 
    {  
      printf("%s\n", "An error has occurred.");    
      return EXIT_FAILURE;
    }
}

I already removed the C++ swap code I coded from my toolkit

To swap two areas of memory there is nothing that says the temporary storage has to be the same size as what is being swapped.

You could (say) create a buffer that is a single char, and use a loop to swap individual characters in x and y.

Or use templates (with added bonus of typesafety).

With the right checks to ensure that only POD types are passed, there is some theretical advantage to using memcpy. in C++11, there is std::is_trivially_copyable for this. But generally this swap is a useless micro-optimization.

Originally Posted by iMalc

2. It uses return values to indicate success or failure instead of exceptions.

On this, just to mention that new throws an bad alloc exception it doesn't return nullptr on error like malloc does. You can use

Code:

Type *t = new(std::nothrow) Type;

to get the no-throw version that will return nullptr like malloc but it the exception seems easier as if you had 10 swaps you would need to check all those 10 swaps and probably won't be able to handle this case anyway

Originally Posted by C_ntua

On this, just to mention that new throws an bad alloc exception it doesn't return nullptr on error like malloc does. You can use

Code:

Type *t = new(std::nothrow) Type;

to get the no-throw version that will return nullptr like malloc but it the exception seems easier as if you had 10 swaps you would need to check all those 10 swaps and probably won't be able to handle this case anyway

Good point, let me rephrase that:
2. It tries (and fails) to use return values to indicate success or failure, instead of exceptions.