Is it a bad idea to replace POD C-style array with

Question

I'm working with a code base that is poorly written and has a lot of memory leaks.

It uses a lot of structs that contains raw pointers, which are mostly used as dynamic arrays.

Although the structs are often passed between functions, the allocation and deallocation of those pointers are placed at random places and cannot be easily tracked/reasoned/understood.

I changed some of them to classes and those pointers to be RAIIed by the classes themselves. They works well and don't look very ugly except that I banned copy-construct and copy-assignment of those classes simply because I don't want to spend time implementing them.

Now I'm thinking, am I re-inventing the wheel? Why don't I replace C-style array with std:array or std::valarray?

I would prefer std::valarray because it uses heap memory and RAIIed. And std::array is not (yet) available in my development environment.

Edit1: Another plus of std::array is that the majority of those dynamic arrays are POD (mostly int16_t, int32_t, and float) arrays, and the numeric API can possibility make life easier.

Is there anything that I need to be aware of before I start?

One I can think of is that there might not be an easy way to convert std::valarray or std::array back to C-style arrays, and part of our code does uses pointer arithmetic and need data to be presented as plain C-style arrays.

Anything else?

EDIT 2

I came across this question recently. A VERY BAD thing about std::valarray is that it's not safely copy-assignable until C++11.

As is quoted in that answer, in C++03 and earlier, it's UB if source and destination are of different sizes.

Answer 1

I would prefer std::vector as the replacement of c-style arrays. You can have a direct access to the underlying data (something like bare pointers) via .data():

Returns pointer to the underlying array serving as element storage.

Answer 2

The standard replacement of C-style array would be std::vector. std::valarray is some "weird" math-vector for doing number-calculation-like stuff. It is not really designed to store an array of arbitrary objects.

That being said, using std::vector is most likely a very good idea. It would fix your leaks, use the heap, is resizable, has great exception-safety and so on.

It also guarantees that the data is stored in one contiguous block of memory. You can get a pointer to said block with the data() member function or, if you are pre-C++11, with &v[0] for a non-empty vector v. You can then do your pointer business with it as usual.

Answer 3

std::unique_ptr<int[]> is close to a drop-in replacement for an owning int*. It has the nice property that it will not implicitly copy itself, but it will implicitly move.

An operation that copies will generate compile time errors, instead of run time inefficiency.

It also has next to no run time overhead over that owning int* other than a null-check at destruction. It uses no more space than an int*.

std::vector<int> stores 3 pointers and implicitly copies (which can be expensive, and does not match your existing code behavior).

I would start with std::unique_ptr<int[]> as a first pass and get it working. I might transition some code over to std::vector<int> after I decide that intelligent buffer management is worth it.

Actually, as a first pass, I'd look for memcpy and memset and similar functions and make sure they aren't operating on the structures in question before I start adding RAII members.

A std::unique_ptr<int[]> means that the default created destructor for a struct will do the RAII cleanup for you without having to write any new code.