C++ pass by reference returning junk or in wrong variable

Question

I'm pulling pixel data from an image using the CImg library and placing it in an array in a struct:

int JpgHandle::loadJpg(const char* Name, SImageData & data){

    CImg<unsigned long> src(Name);
    int width = src.width();
    int height = src.height();
    unsigned long * pixels = new unsigned long [width*height];
    for (int r = 0; r < height; r++){
        for (int c = 0; c < width; c++){
            unsigned long pixel = (src(c,r,0,2) ) |
                                  (src(c,r,0,1) <<  8) |
                                  (src(c,r,0,0) << 16);
            *(pixels++) = pixel;
        }
    }

    data.pData = pixels;
    data.lWidth = (long) width;
    data.lHeight = (long) height;
    data.lStride = (long) width;

    return 1;
}

SImageData is defined thus:

struct SImageData {
    unsigned long *pData;
    long lWidth;
    long lHeight;
    long lStride;

    SImageData() : pData(NULL), lWidth(0), lHeight(0), lStride(0) {}
};

When I call loadJpg from main like this:

JpgHandle handler;
SImageData data1;

handler.loadJpg(argv[1], data1);

cout << "data1: ";
for(int i = 0; i < 10; i++){
    cout << hex << data1.pData[i] << ", ";
}
cout << "\n";

It returns junk:

data1: 0, 1eff1, 7f226014e798, 7f226014e798, 0, 0, 0, 0, 0, 0,

However, if I do this in main:

JpgHandle handler;
SImageData data1;
SImageData data2;

handler.loadJpg(argv[1], data1);
handler.loadJpg(argv[2], data2);

cout << "data1: ";
for(int i = 0; i < 10; i++){
    cout << hex << data1.pData[i] << ", ";
}
cout << "\n";

cout << "data2: ";
for(int i = 0; i < 10; i++){
    cout << hex << data2.pData[i] << ", ";
}
cout << "\n";

The result is that data1 contains the data from argv[2] and data2 contains junk:

data1: 0, 2011, a0a0a, a0a0a, a0a0a, 90909, 90909, a0a0a, b0b0b, b0b0b, 
data2: 0, 7f0d7d712798, 0, 0, 0, 0, 0, 0, 0, 0,

What gives?

You are incrementing the `pixels` pointer inside those nested loops so it no longer points to the start of the allocated memory block when you assign it to `data.pData`. — Blastfurnace, Sep 13 '13 at 00:13
Ah, I see, so I have to have a pointer pointing to the start of that array so I can pass that to data.pData? — omnipotato, Sep 13 '13 at 00:15
For a total hack you can just `data.pData = pixels - (width*height);` , or `unsigned long *pixels = data.pData = new unsigned long [width*height];` and forego the assignment after the loops. — WhozCraig, Sep 13 '13 at 00:24

score 2 · Accepted Answer · answered Sep 13 '13 at 00:17

2

You should save the pointer to allocated memory into data.pData before you enter the nested loops. The loops increment pixels so it no longer points the the start of the memory block.

Do you ever call delete[] on data.pData? That should have triggered a fault because it wasn't the same address returned from new[].

answered Sep 13 '13 at 00:17

Blastfurnace

18,411
56
55
70

Thank you, that fixed my problem. No I'm not calling delete[], but it didn't give any runtime errors. – omnipotato Sep 13 '13 at 00:21
@omnipotato: The bad pointer also meant you were accessing memory outside the allocated block. You just got unlucky that it didn't fail in an obvious way. – Blastfurnace Sep 13 '13 at 00:23
@omnipotato leak memory enough times, I assure you you'll have plenty of runtime errors. Clean up properly, or better still, use a `std::vector` and let it do all the housekeeping for you. (and +1, Mr Furnace =P) – WhozCraig Sep 13 '13 at 00:23
I'm using someone else's code that takes a pointer to a unsigned long array, or else I probably would. – omnipotato Sep 13 '13 at 00:26
1

@omnipotato their API takes an `unsigned long*` ? You can *still* use a `std::vector`. Pass the [`data()`](http://en.cppreference.com/w/cpp/container/vector/data) member result to the API requiring the pointer (assuming you properly sized and filled your vector, of course). – WhozCraig Sep 13 '13 at 00:27

C++ pass by reference returning junk or in wrong variable

1 Answers1