Unusual shared_ptr use in copy_file

[nyalloc]

The copy_file function uses shared_ptr as a RAII wrapper around close through a custom deleter.

filesystem/include/ghc/filesystem.hpp

Line 3510 in 3f1c185

std::shared_ptr<void> guard_in(nullptr, [in](void*) { ::close(in); });

filesystem/include/ghc/filesystem.hpp

Line 3519 in 3f1c185

std::shared_ptr<void> guard_out(nullptr, [out](void*) { ::close(out); });

It seems like this code is using shared_ptr as a mechanism for RAII. shared_ptr isn't necessarily free: it will likely allocate storage for the lamdas and their captures because it has to type erase the custom deleter internally. I find it confusing to see shared_ptr used this way. This is not really its intended use, so the intent of the code wasn't really clear on my first read.

I don't see why RAII is advantageous here: it would make sense if any of the functions called in copy_file could throw, because the destructor would ensure close is called. However, the function is noexcept. If anything throws in here, the application will be terminated anyway.

This is just something I noticed while I was digging around the source. I could submit a PR addressing these comments if you agree with them. Otherwise, I'd be really interested to hear why the code is written like this.

Cheers!

[gulrak]

You are right, this isn't the best place to use this pattern because of the noexcept. The reason it's in there is, that (as written in the readme under "Motivation") this library was not started in 2018 from scratch but I used a filesystem library I wrote in 2011 that much more loosely followed the boost::filesystem interface, when I tried to make my projects less third-party-depending.
It was part of a project to get deeper into C++11, and I used the "shared_ptr for RAII" pattern more often that time. The code was not noexcept at that time either, as I didn't have an error_code version at that time, so it used exceptions from inside the copy loop.

When reworking that code base into a std::filesystem api-conforming one, I simply didn't refactored that one out. And actually there was even a bug from that reworking, as originally the shared pointer order was swapped and and a a PR (#20) at least fixed that. Overall the code is not as homogeneous as I would wish, as it is from almost a decade of living code and style changes over the years. I hope to do some cleanup rounds over time, but I'm still quite happy considering the amount of work needed to complete the compatibility level I strived to achieve.

So yeah, it works, but I should clean that up. And a PR for that will be gladly integrated, as I'm currently quite busy at my job. In any way, thanks for looking through it! I'm always thankful for input to make this library a better piece of helpful software.

[nyalloc]

Thanks for the explanation! I understand, libraries like this often have colourful histories. I'll gladly have a look into this and draft a PR when I have some free time.

[gulrak]

Rework released with v1.3.4