Rotating and printing an image

Question

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 <iostream>
#include <initializer_list>
#include <memory>

using namespace std;
using Ptr2D = unique_ptr<unique_ptr<int[]>[]>;
using IniList = initializer_list<int>;

void printImage(const Ptr2D& image, const IniList& il)
{
    for (auto& i : il)
    {
        for (auto& j : il)
        {
            cout << image[i][j] << ',';
        }

        cout << '\n';
    }
}

template<size_t SIZE>
void rotateImage(Ptr2D& image, const IniList& il)
{
    Ptr2D temp(new unique_ptr<int[]>[SIZE]);

    for (auto& i : il)
    {
        unique_ptr<int[]> 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<int[]>[SIZE]);

    IniList il = { 0, 1, 2, 3, 4 };

    for (auto& i : il)
    {
        unique_ptr<int[]> temp(new int[SIZE]);

        for (auto& j : il)
        {
            temp[j] = j + il.size() * i;
        }

        image[i] = move(temp);
    }

    cout << "******* original image ******\n";

    printImage(image, il);

    rotateImage<SIZE>(image, il);

    cout << "******* rotated image ******\n";

    printImage(image, il);
}

Answer 1

At a quick glance there doesn't appear to be any memory leaks:

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<int[]> 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.