4

I'm in the process of refactoring some old code. There's a C style function that works like this: (Obviously I've simplified it here)

int LoadData(char** buf1, int* buf1Len, char** buf2, int* buf2Len) {
    *buf1Len = DetermineLength1();
    *buf1 = (char*)malloc(*buf1Len);
    // Fill buf1
    *buf2Len = DetermineLength2();
    *buf2 = (char*)malloc(*buf2Len);
    // Fill buf2
    int result = 0; // Or some other INT depending of result
    return result;
}

Now, I'd like to update this code to somehow return a unique_ptr or equivalent, so the pointer will be automatically managed by the caller, and the caller won't ever forget to free the memory.

I couldn't find a good solution, so currently I've changed the code to the following:

int LoadData(std::unique_ptr<char[]>* ubuf1, int* buf1Len, std::unique_ptr<char[]>* ubuf2, int* buf2Len) {
    // same code as above, and finally:
    ubuf1->reset(buf1);
    ubuf2->reset(buf2);
    return result;
}

This doesn't look good, so I'm looking to see if there's a better solution. Since I'm returning two buffers, using the unique_ptr as the return value is not an option.

Is there any better way for this?

ShayanOH
  • 132
  • 1
  • 10
  • 2
    Any reason you can't use `std::vector` for `buf1` and `buf2`? Pass by reference to `LoadData` which can then simply call `resize` accordingly. – G.M. May 07 '17 at 14:37

1 Answers1

11

Don't use std::unique_ptr for arrays if you don't have to. The proper tool for storing dynamic arrays is usually std::vector. It packs the size information right along with the object, where it belongs. If your char* are being used as strings, then you might want to use std::string instead.

std::pair<std::vector<char>, std::vector<char>>
LoadData()
{
    std::vector<char> buf1(DetermineLength1());
    // Fill buf1

    std::vector<char> buf2(DetermineLength2());
    // Fill buf2

    return { std::move(buf1), std::move(buf2) };
}

If you still need your int return result, then you can change the function to return std::tuple<int, std::vector<char>, std::vector<char>>. Or better yet, create a struct with meaningful names for the members.

Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • Why you say "Don't use std::unique_ptr for arrays if you don't have to."? Any reason to not use unique_ptr here? – Nabi May 09 '17 at 04:14
  • 1
    @Nabi: The size of an array is an intrinsic property of the array. Using a unique_ptr requires you to store the size separately. This is error prone. What advantages do you think a unique_ptr offers over a vector? – Benjamin Lindley May 09 '17 at 06:12
  • @Benjamin_lindley: Yes you right, but using a vector to store a char array buffer needs more memory and its accessibility has less speed from char * or unique_ptr or std::string. I'm convinced the best solution for char array buffer is using std::string and its data() function, but i don't like its name (string) when its data will be some binary bytes. – Nabi May 10 '17 at 06:54
  • 1
    @Nabi I'm not so sure that vector will be slower. See this: http://stackoverflow.com/questions/2524233/speed-accessing-a-stdvector-by-iterator-vs-by-operator-index > The performance difference is likely negligable or none (the compiler might optimise them to be identical); – Liran Funaro May 10 '17 at 11:52
  • @LiranFunaro: No, that link is not related to this comparation. In some cases char array is very faster to use, like bulk copying. – Nabi May 13 '17 at 04:38
  • @ BenjaminLindley and @LiranFunaro, Yes, you right! now I think the best solution is using vector and its performance would be same as char *. Thanks! – Nabi Dec 24 '20 at 01:59