jatinchowdhury18 / RTNeural

Real-time neural network inferencing
BSD 3-Clause "New" or "Revised" License
543 stars 57 forks source link

Examples contain undefined behavior #140

Open nvssynthesis opened 3 weeks ago

nvssynthesis commented 3 weeks ago

In the "getModelFile" function, within the loop while((--path.end())->string() != "RTNeural") path = path.parent_path();, you are dereferencing an iterator with (--path.end())->string(). According to cppreference, "Dereferencing [the end] iterator is undefined behavior". I believe this led to a bug at runtime using Clang.

The implementation of getModelFile I ended up replacing it with (for now) looks like:

const std::string js_fn = "rt_models/my_model.json";
const std::string project_root = "RTNeural";

std::string getModelFile(fs::path path) {
    path = fs::absolute(path);

    std::vector<fs::path> components;

    while (!path.empty() && path.filename() != project_root) {
        components.push_back(path.filename());
        path = path.parent_path();
    }
    if (path.empty()) {
        throw std::runtime_error("Root directory not found");
    }

    fs::path resultPath = path / js_fn;

    return resultPath.string();
}
jatinchowdhury18 commented 3 weeks ago

Good catch!

I do like your solution, but I'd prefer to keep the example code as focused as possible. With that in mind, I think it's best to keep the getModelFile() method minimal, even if it's not "bulletproof".

That said, potentially de-referencing path.end() isn't great, especially since implementations of the STL might flag it with a run-time assertion.

How's this as a minimal but better solution:

// get path of RTNeural root directory
while(path.filename() != "RTNeural")
    path = path.parent_path();
nvssynthesis commented 1 day ago

Yeah, makes sense to keep it simple. That works!