Correct way to handle error in boost::filesystem::recursive_directory_iterator::increment()?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Correct way to handle error in boost::filesystem::recursive_directory_iterator::increment()?

Boost - Users mailing list
Hi

I see code like this in an application to iterate over
files in a directory with boost::filesystem (v1.70) without
using exceptions:

    boost::filesystem::path dir("somedir");
    boost::system::error_code error;
    boost::filesystem::directory_iterator node(dir, error), end;

    if (error) {
      return ...;
    }
    for (; node != end; node.increment(error)) {
      const boost::filesystem::path& path = node->path();
      // ...
    }

It appears to work... or mostly work since a crash
was reported and the stack points to this area of the
code. It's a rare crash that I could not reproduce myself.

I suspect that there is a bug in the above code and it
should instead be:

    boost::filesystem::path dir("somedir");
    boost::system::error_code error;
    boost::filesystem::directory_iterator node(dir, error), end;

    for (; !error && node != end; node.increment(error)) {
      const boost::filesystem::path& path = node->path();
      // ...
    }

In other words, I suspect that we need to check that
node.increment(error) in the for loop did not give an error
before checking the condition "node != end", or else we
may enter the loop in case of error and then it may access
node->path() with an invalid node iterator.

I could not find in the doc at ...
  https://www.boost.org/doc/libs/1_75_0/libs/filesystem/doc/reference.html
... what exactly happens when node.increment(error) gives
an error. Perhaps the doc deserves clarification or an example
illustrating how to reliably iterate over files without using
exceptions would be welcome.

Thanks
Dominique
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: Correct way to handle error in boost::filesystem::recursive_directory_iterator::increment()?

Boost - Users mailing list
Dominique Pellé wrote:

> Hi
>
> I see code like this in an application to iterate over
> files in a directory with boost::filesystem (v1.70) without
> using exceptions:
>
>     boost::filesystem::path dir("somedir");
>     boost::system::error_code error;
>     boost::filesystem::directory_iterator node(dir, error), end;
>
>     if (error) {
>       return ...;
>     }
>     for (; node != end; node.increment(error)) {
>       const boost::filesystem::path& path = node->path();
>       // ...
>     }
>
> It appears to work... or mostly work since a crash
> was reported and the stack points to this area of the
> code. It's a rare crash that I could not reproduce myself.
>
> I suspect that there is a bug in the above code and it
> should instead be:
>
>     boost::filesystem::path dir("somedir");
>     boost::system::error_code error;
>     boost::filesystem::directory_iterator node(dir, error), end;
>
>     for (; !error && node != end; node.increment(error)) {
>       const boost::filesystem::path& path = node->path();
>       // ...
>     }
>
> In other words, I suspect that we need to check that
> node.increment(error) in the for loop did not give an error
> before checking the condition "node != end", or else we
> may enter the loop in case of error and then it may access
> node->path() with an invalid node iterator.
>
> I could not find in the doc at ...
>   https://www.boost.org/doc/libs/1_75_0/libs/filesystem/doc/reference.html
> ... what exactly happens when node.increment(error) gives
> an error. Perhaps the doc deserves clarification or an example
> illustrating how to reliably iterate over files without using
> exceptions would be welcome.

Replying to self: stumbling upon ticket
https://github.com/boostorg/filesystem/issues/112
the last comment there indicates that it's undefined
whether the iterator should progress or not in case
of error.  So I think my fix is correct.  I.e.

  for (; node != end; node.increment(error)) { ...}
... should be:
  for (; !error && node != end; node.increment(error)) { ...}

The documentation deserves clarification in my opinion.

Regards
Dominique
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users