CODEWITHSUNDEEP
c++pointersencryptionarrays
ali
ali

Reputation: 11045

Simple C++ char array encryption function - Segment fault

As always, problems with the pointers. I am trying to create a very simple "encryption/decryption" function for char arrays. Yes, I know I can use strings, but I want to improve my knowledge about pointers and make use of simple bytes to achieve a simple task. So, I created a simple struct like this:

struct text {
    char* value;
    int size;
}

And I created this simple function:

text encrypt(text decrypted) {
    char key = 'X';
    for (int i=0; i<decrypted.size; i++) {
        decrypted.value[i] = decrypted.value[i] ^ (key + i) % 255);
    }
    return decrypted;
}

At this point, an experienced C++ programmer should have spot the problem, I think. Anyway, I call this function like this:

...
text mytext;
mytext.value = new char[5];
mytext.value = "Hello";
mytext.size = 5;

mytext = encrypt(mytext);
...

I get, like always, a 'Segmentation fault(core dumped)' error. This is Linux, and, of course, g++. What have I done, again? Thanks!

Upvotes: 0

Views: 2546

Answers (2)

Daniel Fischer
Daniel Fischer

Reputation: 183978

mytext.value = new char[5];
mytext.value = "Hello";

on the second line, you throw away the (handle to the) allocated memory, leaking it, and let mytext.value point to a string literal. Modifying a string literal is undefined behaviour, and usually crashes, since string literals are often stored in a read-only memory segment.

If you insist on using a char*, you should strncpy the string into the allocated memory (but be aware that it won't be 0-terminated then, you should better allocate a new char[6] and copy also the 0-terminator).

Or let decrypt create a new text that it returns:

text encrypt(text decrypted) {
    char key = 'X';
    text encrypted;
    encrypted.size = decrypted.size;
    encrypted.value = new char[encrypted.size];
    for (int i=0; i<decrypted.size; i++) {
        encrypted.value[i] = decrypted.value[i] ^ (key + i) % 255;
    }
    // What about 0-terminators?
    return encrypted;
}

But, as you're using C++, std::string would be a better choice here.

Upvotes: 3

Luchian Grigore
Luchian Grigore

Reputation: 258648

You're modifying string literals:

mytext.value = "Hello";

after this, you can no longer legally mutate what mytext.value points to, you can only re-assign the pointer.

The fix: use std::string

Upvotes: 3

Related Questions