-2

In the following code I call the editorOpen method with a file name. It always makes a segmentation fault except for when I comment out the first line in editorUpdateRow (std::string rowStr(row);). Can someone explain why that happens?

void editorUpdateRow(const std::string& row)
{
    std::string rowStr(row);
    size_t index = 0;
    while (index != std::string::npos)
    {
        index = rowStr.find("\t", index);
        if (index != std::string::npos)
        {    
            rowStr.replace(index, 1, KILO_TAB);
            index += sizeof(KILO_TAB) - 1;
        }
    }
}

void editorAppendRow(char* row, size_t len)
{
    config.rows = static_cast<std::string*>(realloc(config.rows, sizeof(std::string) * (config.numRows + 1)));
    KILO_SANITY(config.rows != nullptr, "Couldn't realloc rows array");
    printf("%d\n", config.numRows);
    config.rows[config.numRows] = row;
    printf("%d\n", __LINE__);
    config.rows[config.numRows][len] = '\0';
    printf("%d\n", __LINE__);
    editorUpdateRow(config.rows[config.numRows]);

    ++config.numRows;
}

void editorOpen(char* filename)
{
    KILO_SANITY(filename != nullptr, "Filename is NULL!!!");
    FILE* fp = fopen(filename, "r");
    KILO_SANITY(fp, "Open of %s Failed!!!", filename);
    char* line = nullptr;
    size_t linecap = 0;
    ssize_t linelen = 0;
    while((linelen = getline(&line, &linecap, fp)) != -1)
    {
            while (linelen > 0 && (line[linelen - 1] == '\n' ||
                   line[linelen - 1] == '\r'))
            {
                --linelen;
            }
            editorAppendRow(line, linelen);
    }

    free(line);
    fclose(fp);
}
  • 1
    `malloc`/`calloc`/`realloc` don't work with `std::string`s. Use `new` instead. – HolyBlackCat Nov 18 '17 at 18:52
  • Better still, don't explicitly dynamically allocate strings - there is almost never any need to. –  Nov 18 '17 at 19:00
  • Put away the `C` coding and use`std::vector`. All of those calls to `free` and `realloc` will not work. A `std::string` must be properly constructed, and those functions know nothing about C++ and object construction. – PaulMcKenzie Nov 18 '17 at 19:04

1 Answers1

1

A std::string is a non-trivial class that requires proper construction to function. malloc simply allocates memory. It does not run the constructor, so the allocated string's state is uninitialized. The string's internal character buffer points Crom knows where, its length is undefined, and whatever other bookkeeping is used in the string's implementation is not set to meaningful values. A malloced string is a bomb waiting to go off.

Because malloc is dangerous to use with complex data types (and because it's possible to get the size wrong), it should almost never be used in C++.

When forced to allocate dynamic storage, prefer (in order)

  1. a library container
  2. std::make_unique
  3. std::make_shared
  4. new

Pick the earliest one in the list that fits your object's requirements.

By itself, string almost is a library container, a container of characters, so dynamically allocating a string is almost never the correct option. You can pass it around by reference and transfer ownership with std::move if you wish to avoid copying.

user4581301
  • 33,082
  • 7
  • 33
  • 54