Debug error: stack around the variable 'cardDesc' was corrupted

Question

TITLE. The function i'm using is this:

void GetVideoCardInfo(int* memoryVar, char* cardName)
{
    strcpy_s(cardName, 128, m_videoCardDescription);
    *memoryVar = m_videoCardMemory;
    return;
}

m_videoCardDescription is a '128 character long' character array that has the description of my video card in it. Here is where I am calling the function:

bool writeGPUnameDesc() {
    char cardDesc;
    int cardMem;
    m_D3D->GetVideoCardInfo(&cardMem, &cardDesc);

    std::ofstream myfile;
    myfile.open("gpuNameAndDesc.txt");
    myfile << "Graphics card name: " << cardDesc;
    myfile << " - Graphics card memory: " << cardMem;
    myfile.close();

    return true;
}

When I run the program a message box pops up that says Runtime-check failure #2 and the title. If anyone can help thanks in advance.

`cardDesc` has room for exactly one character. You read 128 characters into that and you're gonna have a bad time. — user4581301, Nov 21 '18 at 23:20
Is there a way I can make it able to store 128 characters but still work as a pointer? — Max, Nov 21 '18 at 23:23
Why are you messing with C-style strings in C++? What's the problem with `std::string`? — eesiraed, Nov 21 '18 at 23:30

score 5 · Accepted Answer · answered Nov 21 '18 at 23:25

5

You are copying 128 characters into char cardDesc, which represents only 1 character.

You should change the type of cardDesc to a char-array:

char cardDesc[128];
// ...
m_D3D->GetVideoCardInfo(&cardMem, cardDesc);
//                                ^ no &

answered Nov 21 '18 at 23:25

Martin Heralecký

5,649
3
27
65

Where does the null terminator go? – user4581301 Nov 21 '18 at 23:35
@user4581301 inside `cardDesc`, according to the documentation of `strcpy_s` – M.M Nov 22 '18 at 00:10
Can't argue that. What I'm worried about is is the originator 128 characters plus terminator or 128 including terminator. Could be dropping a character. – user4581301 Nov 22 '18 at 00:13

user4581301 · Answer 2 · 2018-11-21T23:40:08.310

TL;DR

std::string GetVideoCardInfo(int & memoryVar)
{
    memoryVar = m_videoCardMemory;
    return m_videoCardDescription;;
}


bool writeGPUnameDesc() {
    int cardMem;
    std::string cardDesc = m_D3D->GetVideoCardInfo(cardMem); 

    std::ofstream myfile;
    myfile.open("gpuNameAndDesc.txt");
    myfile << "Graphics card name: " << cardDesc;
    myfile << " - Graphics card memory: " << cardMem;
    myfile.close();

    return true;
}

Explanation

strcpy_s(cardName, 128, m_videoCardDescription);

is a blatant lie. The size of cardName is exactly one character. If you lie to strcpy_s its extra checking to make sure you don't overrun the buffer cannot help you.

Inferior solutions

Replace

char cardDesc;

with

char cardDesc[129];

The better approach gets rid of the magic numbers entirely.

Up near the top of the file

namespace // annonymous namespace. Contents will not leak out of the file
{
    constexpr int MAX_CARD_NAME_LENGTH = 129 // 128 plus room for nul terminator
}

and then

void GetVideoCardInfo(int* memoryVar, char* cardName)
{
    strcpy_s(cardName, MAX_CARD_NAME_LENGTH, m_videoCardDescription);
    *memoryVar = m_videoCardMemory;
    return;
}

and

bool writeGPUnameDesc() {
    char cardDesc[MAX_CARD_NAME_LENGTH]; // now an array
    int cardMem;
    m_D3D->GetVideoCardInfo(&cardMem, cardDesc); // don't need to take address of array

    std::ofstream myfile;
    myfile.open("gpuNameAndDesc.txt");
    myfile << "Graphics card name: " << cardDesc;
    myfile << " - Graphics card memory: " << cardMem;
    myfile.close();

    return true;
}

Debug error: stack around the variable 'cardDesc' was corrupted

2 Answers2

TL;DR

Explanation

Inferior solutions