patterncppMinor
Rotating and printing an image
Viewed 0 times
andprintingrotatingimage
Problem
I carefully used smart pointer to avoid memory leaks and dangling pointers, but someone says my code has memory leaks. Is it true? If so, how can I fix it?
#include
#include
#include
using namespace std;
using Ptr2D = unique_ptr[]>;
using IniList = initializer_list;
void printImage(const Ptr2D& image, const IniList& il)
{
for (auto& i : il)
{
for (auto& j : il)
{
cout
void rotateImage(Ptr2D& image, const IniList& il)
{
Ptr2D temp(new unique_ptr[SIZE]);
for (auto& i : il)
{
unique_ptr subTemp(new int[SIZE]);
for (auto& j : il)
{
subTemp[j] = 0;
}
temp[i] = move(subTemp);
}
for (auto& i : il)
{
for (auto& j : il)
{
temp[i][j] = image[SIZE - j - 1][i];
}
}
for (auto& i : il)
{
for (auto& j : il)
{
image[i][j] = move(temp[i][j]);
}
}
}
int main()
{
const size_t SIZE = 5;
Ptr2D image(new unique_ptr[SIZE]);
IniList il = { 0, 1, 2, 3, 4 };
for (auto& i : il)
{
unique_ptr temp(new int[SIZE]);
for (auto& j : il)
{
temp[j] = j + il.size() * i;
}
image[i] = move(temp);
}
cout (image, il);
cout << "******* rotated image ******\n";
printImage(image, il);
}Solution
At a quick glance there doesn't appear to be any memory leaks:
Valgrind appears to agree about the lack of memory leaks.
I would just use
Assuming we changed this to use
valgrind ./mem_leak_test
==3914== Memcheck, a memory error detector
==3914== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==3914== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==3914== Command: ./mem_leak
==3914==
* original image ****
0,1,2,3,4,
5,6,7,8,9,
10,11,12,13,14,
15,16,17,18,19,
20,21,22,23,24,
* rotated image ****
20,15,10,5,0,
21,16,11,6,1,
22,17,12,7,2,
23,18,13,8,3,
24,19,14,9,4,
==3914==
==3914== HEAP SUMMARY:
==3914== in use at exit: 0 bytes in 0 blocks
==3914== total heap usage: 12 allocs, 12 frees, 296 bytes allocated
==3914==
==3914== All heap blocks were freed -- no leaks are possible
==3914==
==3914== For counts of detected and suppressed errors, rerun with: -v
==3914== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)
Valgrind appears to agree about the lack of memory leaks.
I would just use
std::array instead of code such as unique_ptr temp(new int[SIZE]);. You get the benefits of automatic memory cleanup at the end of the variables scope from std::array without as much complexity. The main reason I would go for std::array is because you appear to be mostly concerned with memory management and not ownership. However if your intent here is to clearly have single ownership then unique_ptr is a good choice.std::array will reduce complexity because it contains information about its dimensions. This would completely remove the need for the initializer list for initializing the array. Unless you are going to do something more fancy with it, the initializer list is an overly complex way to iterate over all the indexes of an array.Assuming we changed this to use
std::array the other thing I would do differently is to return the rotated image from rotateImage as opposed to modifying a reference parameter. Fewer side effects tends to keep your code more conceptually simple.Context
StackExchange Code Review Q#73777, answer score: 5
Revisions (0)
No revisions yet.