6

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.

Community
  • 1
  • 1
user3528438
  • 2,737
  • 2
  • 23
  • 42
  • Unless I'm missing it, I'm surprised `std::valarray` doesn't have a `.data()` like `std::vector` that exposes the underlying raw array for when you need a C-style array. – Cory Kramer Jan 30 '15 at 14:23
  • 6
    From the description you have, it seems a std::vector should be suitable. Is there anything in particular that warrants you to consider std::valarray? – nos Jan 30 '15 at 14:24
  • 2
    `std::valarray` is/was sort of meant for LAPACKish applications; it's rarely used and probably not a good fit here. `std::vector` is the usual container for dynamic arrays, and you can get a pointer to its data with `&vec[0]` pre-C++11 (when `.data()` was not yet available). – Wintermute Jan 30 '15 at 14:26
  • 1
    @nos A plus of std::valarray is its numeric API is very close to numpy and matlab. And the majority of the arrays I use are POD arrays. – user3528438 Jan 30 '15 at 14:39
  • @nos Probability the numeric API, and the majority of data being POD instead of objects. – user3528438 Jan 30 '15 at 14:44
  • @user3528438 if you need the numeric API, that's a good reason. Otherwise a std::vector would imo. be more suitable - a vector is suitable for POD data and e.g simple integer types as well. – nos Jan 30 '15 at 14:50
  • 2
    @Cyber: One of the goals of `valarray` was to ensure against aliasing, which translated fairly directly to prohibiting access to any pointer to the actual data. – Jerry Coffin Jan 30 '15 at 16:47

3 Answers3

13

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.

Baum mit Augen
  • 49,044
  • 25
  • 144
  • 182
  • Not to forget that using `std::vector`, you'll also get copy semantics without doing anything. No need to `delete` copy construrctor and assignment operator any more. – 5gon12eder Jan 30 '15 at 14:55
  • @5gon12eder Yes, I thought about that, too. But in the long run I'll still ban them for performance reasons. It's not good practice to deep copy large numeric arrays anyway, and that's (one of the reasons) why MATLAB is slow. – user3528438 Jan 30 '15 at 15:17
  • 3
    @user3528438 Not copying is (almost) always faster than copying but if you *have to* copy, using the highly optimized implementation in `std::vector` comes in handy. – 5gon12eder Jan 30 '15 at 15:19
6

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.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Can you further explain why you'd look for memcpy and memset? Is it because memcpy(struct *to, struct *from, sizeof(struct)) is very unsafe when struct is transformed to an RAII class? – user3528438 Jan 30 '15 at 15:07
  • 2
    @user3528438 yes. `memset` to clear, `memcpy` to copy, etc -- all are common microoptimizations in C-style code, and will be fatally damaging to RAII classes (in practice) and undefined behavior (in theory). – Yakk - Adam Nevraumont Jan 30 '15 at 15:10
5

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.

masoud
  • 55,379
  • 16
  • 141
  • 208