Search code examples
c++segmentation-faultinitializationtypedef

Trying to initialize class objects inside a struct is causing segmentation faults


I am attempting a program using C++ that prints text to a terminal in different colors.

I have the following code which features the Color class used inside the letter_template struct, which are the elements of an array of size 100:

#include <ctype.h>
#include <stdio.h>
#include <unistd.h>

// Works on little endian processors which mine happens to be
#define COLOR_PREPEND 0x001b;

class Color
{
private:
    char contents[6];
public:
    Color() {
        short prepend = COLOR_PREPEND;
        sprintf(contents, "%s[%dm", (char *)prepend, 0);
    }
    Color(int code) {
        short prepend = COLOR_PREPEND;
        sprintf(contents, "%s[%dm", (char *)prepend, code);
    }
};

typedef struct {
    Color *set;
    char c;
    Color *reset;
} letter_template;

letter_template essay[100];

int main() {
    // Initialize array
    for (int i = 0; i < sizeof(essay)/sizeof(essay[0]); i++) {
        essay[i].set = new Color(); essay[i].reset = new Color();
    }

    return 0;
}

The segfault occurs on the first iteration of the for loop. Using a debugger, I noticed that the contents of every member of essay had null values (0x0) for both the set and reset attributes, so I guess it couldn't initialize the Color objects. I cannot figure out why, though.

At first, what I was really trying to do was to have one long contiguous string of data in the order of color sequence > letter > color sequence > repeat. I know that what I have coded will not be exactly that, however I am at a loss for why it shouldn't be working, at least.

I am stumped because my Color class definition, as you see it, worked just fine before I put it into the class and typedef structure (as in, the sprintf statement, as you see it, worked fine when I just had it written in main). My contents attribute has 6 bytes, which I know is enough space for what I am sprintf'ing there.


EDIT:

I updated the code to exactly what I have, including the missing defines and includes. My compile command was:

g++ -c main.cpp -o main.o; g++ main.o -o main.exe

Same issue as before.

Note: It was C code originally, which is why it looks like C code. The reason I am using objects and C++ things is because I wanted the utility of being able to set and reset specific colors on specific letters without hacking hard-coded strings everywhere, for which I wanted to use classes and methods.


Solution

  • Your code exhibits undefined behavior, so anything is possible, and all bets are off.

    In your sprintf() calls, the %s specifier expects a char* pointer to a null-terminated character string, but you are instead giving it a char* pointer that has been type-casted from a short integer value. The pointer is not pointing at valid memory for sprintf to read from, thus it crashes.

    You probably meant to type-cast the address of the short rather than its value, eg:

    class Color
    {
    private:
        char contents[6];
    public:
        Color() {
            setCode(0);
        }
        Color(int code) {
            setCode(code);
        }
        void setCode(int code) {
            short prepend = COLOR_PREPEND;
            sprintf(contents, "%s[%dm", (char *)&prepend, code);
    };
    

    On a little-endian system, this would happen to "work" to insert a 0x1B byte into your contents buffer. But this is simply the wrong approach and should not be done this way to begin with. For what you are attempting to do, use %c instead of %s, eg:

    static const char COLOR_PREPEND = 0x1b;
    
    class Color
    {
    private:
        char contents[6];
    public:
        Color() {
            setCode(0);
        }
        Color(int code) {
            setCode(code);
        }
        void setCode(int code) {
            sprintf(contents, "%c[%dm", COLOR_PREPEND, code);
    };
    

    Or, you could simply hard-code the char directly in your format string:

    class Color
    {
    private:
        char contents[6];
    public:
        Color() {
            setCode(0);
        }
        Color(int code) {
            setCode(code);
        }
        void setCode(int code) {
            sprintf(contents, "\x1b[%dm", code);
    };
    

    Either way, do note that you still have a potential buffer overflow waiting to happen, if code ever contains a value < -9 or > 99.

    At the very least, you should take in the code as an (unsigned) char instead of a int, and do proper bound checking on its value. Otherwise, increase the size of contents to account for the largest int you will ever use.

    Or, simply change contents to a std::string instead, eg:

    static const char COLOR_PREPEND = 0x1B;
    
    class Color
    {
    private:
        std::string contents;
    public:
        Color() {
            setCode(0);
        }
        Color(int code) {
            setCode(code);
        }
        void setCode(int code) {
            contents = std::string(1, COLOR_PREPEND) + "[" + std::to_string(code) + "m";
            // or:
            // contents = "\x1b[" + std::to_string(code) + "m";
        }
    };