How can I replace the C-style string parsing with C++ core guideline-compliant code?

Question

Take for example these two code fragments:

//...
file_string = strstr(file_string, "\nv ");
while (file_string = strstr(file_string, "v ")) {
    vec::vec3<float> buffer = { 0.0f };
    file_string += strlen("v ");
    file_string = std::from_chars(file_string, file_string_end, buffer.x).ptr;
    file_string++;
    file_string = std::from_chars(file_string, file_string_end, buffer.y).ptr;
    file_string++;
    file_string = std::from_chars(file_string, file_string_end, buffer.z).ptr;
    file_string++;
    vcoords.push_back(std::move(buffer));
}
//...

//...
while (file_string = strstr(file_string, "v ")) {
    size++;
    file_string++;
}
vcoords.reserve(size);
//...

For this type of data

(...)
v 1.000000 1.000000 -1.000000
v 1.000000 -1.000000 -1.000000
v 1.000000 1.000000 1.000000
v 1.000000 -1.000000 1.000000
v -1.000000 1.000000 -1.000000
(...)

they work and they work sufficiently quickly. They also generate warnings such as:

C26481: Don't use pointer arithmetic.

C26486: Don't pass a pointer that may be invalid to a function. Parameter 0 'file_string' in call to 'strstr' may be invalid (lifetime.3).

How can I replace the strstr/pointer arithmetic mix with something that would do its job comparably fast and not generate such warnings? I tried going through similar questions related to std::string-to-float conversions but they either used std::stringstream, which is very slow, or assumed that the string in question contained nothing but one value.

That first warning is rather stylistic, asking you to use very recent "fat pointer" types like `std::span` to represent array slices, rather than pointers. You can get around it possibly by using `std::string_view` to represent the string, and its iterators (begin(), etc) to do the arithmetic instead of `char*`. You could also be forgiven for turning taht warning off. The second warning is much more worrisome, as it's doing some static lifetime analysis and thinks you're going to use a reference after the original has died. It's not immediately clear to me what's causing that. — parktomatomi, Nov 11 '19 at 00:42
You don't have to entirely drop `std::string` to use `std::from_chars`. `string::find` returns an integer, which you can convert to a `const char*` with `&file_string[pos]`. You can similarly convert an iterator by dereferencing and rereferencing it: `&*it`. — parktomatomi, Nov 11 '19 at 01:03

parktomatomi · Accepted Answer · 2019-11-11T14:44:51.353

edited for changes after enabling core guidelines checker

I took a crack at using std::from_chars. Fun fact: it doesn't work on gcc or clang for floating point values yet! That... might warrant not using it until the feature is mature.

Things to shut up the guidelines checker:

gsl::at is the cheat code for pointer arithmetic. No warning
The lifetime checker is kind of dumb. It doesn't know a sting literal wrapped by a string_view still has infinite lifetime, but it doesn't flag if you use string view literals (""sv).
Still using string::find for searching, then adding the position to string::data()
I recommended indexing instead of pointer arithmetic before, but the analysis doesn't like that either.

using namespace std::literals;
const std::string file_string = "\n"
    "v 1.000000 1.000000 -1.000000\n"
    "v 1.000000 -1.000000 -1.000000\n"
    "v 1.000000 1.000000 1.000000\n";
const auto tag = "v "sv;
const char* file_string_end = &gsl::at(file_string, file_string.size());

std::vector<vec::vec3<float>> vcoords;
std::string::size_type pos = 0;
while ((pos = file_string.find(tag, pos)) != std::string::npos) {
    vec::vec3<float> buffer = { 0.0f };
    auto [x_ptr, x_ec] = std::from_chars(
        &gsl::at(file_string, pos + tag.size()), 
        file_string_end, 
        buffer.x);
    if (x_ec != std::errc()) {
        throw std::runtime_error("bad x");
    }
    std::string_view x_view(x_ptr);
    auto [y_ptr, y_ec] = std::from_chars(
        &gsl::at(x_view, 1), 
        file_string_end, 
        buffer.y);
    if (y_ec != std::errc()) {
        throw std::runtime_error("bad y");
    }
    std::string_view y_view(y_ptr);
    auto [z_ptr, z_ec] = std::from_chars(
        &gsl::at(y_view, 1), 
        file_string_end, 
        buffer.z);
    if (z_ec != std::errc()) {
        throw std::runtime_error("bad z");
    }
    vcoords.push_back(buffer);
}

https://godbolt.org/z/F9j7FA

It doesn't get rid of pointer arithmetic though. The warnings will still be there. — Big Temp, Nov 11 '19 at 11:43
Oh, I see you have some core guidelines checker on. You can use &s[pos] to use the output of find.. You can wrap the pointers ina string_view to use the output from from_chars. I find all this kind of silly and would just turn off the warning if I was using it though. I'll update the answer when I get a chance. — parktomatomi, Nov 11 '19 at 12:42
Ok, I managed to turn on the warnings you're seeing and get them out. Thanks for pointing it out! — parktomatomi, Nov 11 '19 at 14:40

How can I replace the C-style string parsing with C++ core guideline-compliant code?

1 Answers1