[filesystem] Change "remove" function's return to void?

The remove(p) function is currently specified to return a bool with a value of exist(p) before the post-condition is established. The post-condition is !exists(p). The original thought in providing this was that the return value might be useful and would cost nothing. It turns out that providing this return value does have several costs: * A call to exists() could be omitted if the actual system removal call succeeds. exists() is surprisingly expensive on some operating systems. * It makes the remove() function less reliable in that it introduces a potential race condition. * It makes flow of control more complex (and thus harder to test) for some implementations. Thus I'd like to change the return to void. Although that will break some code, the breakage will result in a noisy compile-time error that is easy to fix by inserting a call to exits() before the call to remove(). Comments? --Beman PS: This was precipitated by ticket #1972, but doesn't affect the resolution of the remove() issue it raises. That will be fixed for 1.36.0 regardless.

Beman Dawes wrote:
The remove(p) function is currently specified to return a bool with a value of exist(p) before the post-condition is established. The post-condition is !exists(p).
You can't really guarantee that, can you? Can't some other process sneak in and re-create p before remove returns?
The original thought in providing this was that the return value might be useful and would cost nothing.
It turns out that providing this return value does have several costs:
* A call to exists() could be omitted if the actual system removal call succeeds. exists() is surprisingly expensive on some operating systems.
* It makes the remove() function less reliable in that it introduces a potential race condition.
* It makes flow of control more complex (and thus harder to test) for some implementations.
Thus I'd like to change the return to void. Although that will break some code, the breakage will result in a noisy compile-time error that is easy to fix by inserting a call to exits() before the call to remove().
Comments?
Sounds like you're making the right choice here. -- Dave Abrahams BoostPro Computing http://www.boostpro.com

On Tue, Jul 1, 2008 at 10:40, David Abrahams <dave@boostpro.com> wrote:
Beman Dawes wrote:
The remove(p) function is currently specified to return a bool with a value of exist(p) before the post-condition is established. The post-condition is !exists(p).
You can't really guarantee that, can you? Can't some other process sneak in and re-create p before remove returns?
There's a blanket "Effects and Postconditions not guaranteed in the presence of race-conditions" in effect for the whole library: http://www.boost.org/doc/libs/1_35_0/libs/filesystem/doc/index.htm#Cautions
[...]
Thus I'd like to change the return to void. Although that will break some code, the breakage will result in a noisy compile-time error that is easy to fix by inserting a call to exits() before the call to remove().
Comments?
Sounds like you're making the right choice here.
Sounds good to me too. If there's a potential race there anyways, might as well let the user write it explicitly. ~ Scott

Scott McMurray wrote:
On Tue, Jul 1, 2008 at 10:40, David Abrahams <dave@boostpro.com> wrote:
Beman Dawes wrote:
The remove(p) function is currently specified to return a bool with a value of exist(p) before the post-condition is established. The post-condition is !exists(p). You can't really guarantee that, can you? Can't some other process sneak in and re-create p before remove returns?
There's a blanket "Effects and Postconditions not guaranteed in the presence of race-conditions" in effect for the whole library: http://www.boost.org/doc/libs/1_35_0/libs/filesystem/doc/index.htm#Cautions
Yeah, I know that. My point is, with such a clause in place, what point is there in returning the error code in the first place? -- Dave Abrahams BoostPro Computing http://www.boostpro.com

On Tuesday 01 July 2008 15:54:42 Beman Dawes wrote:
The remove(p) function is currently specified to return a bool with a value of exist(p) before the post-condition is established. The post-condition is !exists(p).
The original thought in providing this was that the return value might be useful and would cost nothing.
It turns out that providing this return value does have several costs:
<snip>
Comments?
I'm not sure why that exists check is done at all. In POSIX at least, remove() fails with specific errno value when the to be removed file does not exist. If the boost::filesystem::remove() semantics want to throw in any error except the "file does not exist" one when it will return a "false" then shouldn't it instead be checking for the errno value for this? Of course if the other platforms do not have something like this then just make remove return void. Also, POSIX errno ENOENT does not exactly mean that the file to be removed does not exist but that "the complete path given to the file" does not exist (which also may mean that some component on the path does not exist). From my point of view, whenever I would check for remove() status if the file that I wanted to be removed existed or not also includes this case so the POSIX semantics are fine with me but probably pretty different from other platforms.
PS: This was precipitated by ticket #1972, but doesn't affect the resolution of the remove() issue it raises. That will be fixed for 1.36.0 regardless.
Goodie, someone is solving #1972, thank you :) -- Mihai RUSU Email: dizzy@roedu.net "Linux is obsolete" -- AST
participants (4)
-
Beman Dawes
-
David Abrahams
-
dizzy
-
Scott McMurray