0

I'm getting an error in the following code. Visual Studio throws an access violation error when writing to _buf. How can I fix this?

The Sendn function is a socket sending function. It's not the problem, you can ignore it.

It looks like _buf points at 0x00000000

The error message I'm seeing is

0xC0000005: 0x00000000 : access violation
void ?????::?????(int number, string title)
{

    int titlesize = sizeof(title);
    int bufsize = 4 + 4 + 4 + titlesize;

    char *_buf = new char[bufsize];

    _buf = { 0 };

    // char _buf[bufsize] = { 0 }; (수정 내용)

    int commands = 3;

    int index = 0;
    memcpy(_buf, &commands, sizeof(int));
    index += sizeof(int);

    memcpy(_buf + index, &number, sizeof(int));
    index += sizeof(int);

    memcpy(_buf + index, &titlesize, sizeof(int));
    index += sizeof(int);
    for (int i = 0; i < titlesize; i++)
    {
        memcpy(_buf + index, &title[i], sizeof(char));
        index += sizeof(char);
    }

    Sendn(_buf, bufsize);

    delete[] _buf;

    return;
}
Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • 1
    What is the reason you are `memcpy()`-ing the `title`, one character at a time, with its own dedicated `for` loop, instead of simply `memcpy()`ing the whole thing, in one shot? – Sam Varshavchik Jul 26 '19 at 01:49
  • Side note, while I think it's not an issue here, as a general rule, [starting symbol names with an underscore is a bad idea](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) because if not done right, it can lead to using some reserved identifiers by accident. –  Jul 26 '19 at 02:03
  • Other side note: instead of `int bufsize = 4 + 4 + 4 + titlesize;`, you should probably do something like `int bufsize = sizeof(int) + sizeof(int) + sizeof(int) + titlesize;`, since [`int` is not guaranteed to be 4 bytes](https://stackoverflow.com/a/35844670/10957435). –  Jul 26 '19 at 02:09
  • 1
    I recommend using [std::copy](https://en.cppreference.com/w/cpp/algorithm/copy) rather than [memcpy](https://en.cppreference.com/w/cpp/string/byte/memcpy) as it is safer and just as fast. – Galik Jul 26 '19 at 02:09

2 Answers2

2
char *_buf = new char[bufsize];
_buf = { 0 };

This does not zero-fill the dynamically-allocated array pointed to by _buf. It sets the pointer _buf to be a null pointer. Since _buf is a null pointer, later attempts to dereference it lead to undefined behavior.

There's no need to zero-fill the array pointed to by _buf in this case, so you can simply remove the _buf = { 0 }; line.


Once you've fixed that problem, you also aren't allocating the right amount of memory. sizeof(title) will not give you the number of characters that title holds. It just gives you the static size of a std::string object, which is usually only a pointer and two integers. Use title.size() instead.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • Fun fact, [`std::fill()`](https://en.cppreference.com/w/cpp/algorithm/fill) will fill an array like this, however. –  Jul 26 '19 at 02:00
0

You're trying to copy the content of title together with 3 other integer numbers into _buf right? The problem is that sizeof(title) is not the length of the string stored in title. In order to get the length of title, you need to call the member function length on type std::string like this:

auto titlesize = title.length();

The sizeof operator only gives you the size of your std::string object on stack (in comparison, the actual string is stored on heap) and sizeof expressions are always constant expressions. On my computer, sizeof(std::string) is 24 regardless of what the actual string is.

Lancern
  • 403
  • 2
  • 9