Search code examples
c++arrayscharcoutdereference

Why am I unable to cout a dereferenced char pointer?


When I try to run the following code, I don't receive any errors or warnings, but my terminal always crashes. Why is this and how do I fix it?

main.cpp

#include <iostream>
#include <cstdio>
#include "colours.hpp"

using namespace std;

int main()
{
    colour co(50,65,78);
    unsigned char *cp = co.getRGB();
    cout << *cp << endl;
    getchar();
    return 0;
}

colours.hpp

#ifndef COLOURS
#define COLOURS

class colour{
    public:
        colour(unsigned char r, unsigned char g, unsigned char b);
        unsigned char *getRGB();
    private:
        unsigned char red, green, blue;
};

#endif // COLOURS

colours.cpp

#include "colours.hpp"

colour::colour(unsigned char r, unsigned char g, unsigned char b) : red(r), green(g), blue(b) {
}

unsigned char *colour::getRGB(){
    unsigned char arr[3] = {red, green, blue};
    return arr;
}

Solution

  • You've defined an array locally on the stack in the following function:

    unsigned char *colour::getRGB(){
        unsigned char arr[3] = {red, green, blue};
        return arr;
    }
    

    When the function returns, the stack is decremented and the array is no longer valid (as is the pointer to the array). Which is the reason your program crashes when you dereference the pointer.

    To fix the problem, you could use an array for the colors, as you know the size at compile time.

    std::array<unsigned char, 3> colour::getRGB(){
        std::array<unsigned char, 3> color = {red, green, blue};
        return color;
    }
    

    Another alternative is to allocate on the heap, instead of the stack,

    unsigned char* colour::getRGB() {
        int size = sizeof(unsigned char) * 3;  // Get the size of 3 unsigned chars.
        unsigned char* color = (unsigned char*) malloc(size);
        color[0] = red;
        color[1] = green;
        color[2] = blue;
    
        return color;
    }
    

    This method comes with a few drawbacks. Firstly, you must remember to free the memory you've allocated when you're done with it.

    colour a = colour(255, 0, 255);
    unsigned char* rgb = a.getRGB();
    // ---- DO STUFF ---
    free(rgb);
    

    Secondly, it is slow. Dynamically allocating memory this ways requires asking the operating system for more memory, which takes a bit of time.

    Thirdly, since we already know that we want to have 3 unsigned chars, it's not necessary to dynamically allocate. Doing so removes the possibility for the compiler to do static analysis and optimizing the code.