Mesh stream read about draco HOT 10 CLOSED

Thanks for the report, the issue has been already fixed.

from draco.

Would be nice to get credit, you didn't fix it until I posted the pull request!

from draco.

Yes. @ondys, you should in future give credit where due. @rtoepfer's pull request, which he submitted yesterday, contains this patch for io/mesh_io.h:

   // Determine size of stream and write into a vector
    auto is_size = is.tellg();
    is.seekg(0, std::ios::end);
 -  is_size = is.tellg() - is_size;
 -  is.seekg(0, std::ios::beg);
 -  std::vector<char> data(is_size);
 -  is.read(&data[0], is_size);
 +  auto vec_size = is.tellg() - is_size;
 +  is.seekg(0, is_size);
 +  std::vector<char> data(vec_size);
 +  is.read(&data[0], vec_size);

Your diff to same, which you wrote half an hour ago, contains what appears to be a rewrite of his patch:

    // Determine size of stream and write into a vector
 -  auto is_size = is.tellg();
 +  const auto start_pos = is.tellg();
    is.seekg(0, std::ios::end);
 -  is_size = is.tellg() - is_size;
 -  is.seekg(0, std::ios::beg);
 +  const std::streampos is_size = is.tellg() - start_pos;
 +  is.seekg(start_pos);
    std::vector<char> data(is_size);
    is.read(&data[0], is_size);

Not cool.

from draco.

It has been actually fixed internally shortly after the report yesterday. It just takes time to propagate the changes to the public repo. In any case, we definitely welcome any external pull requests and we will be happy to merge them as long as they don't conflict with our internal fixes.

from draco.

To be clear, you wrote:

has been actually fixed internally shortly after the report

Did you mean to say "shortly before"? Or did the fix take place after the report, but before you had a chance to read the report?

from draco.

Shortly after. We monitor the issue tracker and any reported bugs are fixed internally first and then they are pushed to the public repo (usually we do the final push only after sufficient number of changes accumulate, unless the fix is considered critical). Note that the pull request in question was posted after about one day after the actual report.

from draco.

from draco.

@ondys are you guys planning to eventually do all development in the open? Otherwise there will be such cases of duplicated work, which discourages contributions.

from draco.

@waldyrious we may eventually move it to open but we don't plan to do it in the near future. To reduce the chance of duplicate work, we will do our best to write a note on issues when they are picked up by internal developers.

from draco.

Understood -- thanks for the clarification.

from draco.