Turn TryCreateDirectory() into TryCreateDirectories() #9895

pull benma wants to merge 1 commits into bitcoin:master from benma:appinitmain changing 6 files +8 −10

benma commented at 2:03 AM on March 1, 2017: none

Use case: TryCreateDirectory(GetDataDir() / "blocks" / "index") would fail if the blocks directory was not explicitly created before.

The line that did so was in a weird location and could be removed as a result.
fanquake added the label Utils and libraries on Mar 1, 2017
benma commented at 2:08 AM on March 1, 2017: none

The blocks directory is now created here:

https://github.com/benma/bitcoin/blob/de85040fc552178815a71cacb00d16faff994c9a/src/dbwrapper.cpp#L52 via https://github.com/benma/bitcoin/blob/11049f4fe62606d1b0380a9ef800ac130f0fbadf/src/txdb.cpp#L70

and is created the first time it is needed with the initialization of CBlockTreeDB in init.cpp.
dcousens approved
dcousens commented at 2:37 AM on March 1, 2017: contributor

utACK
in src/util.cpp:None in de85040fc5 outdated
```
 619 | +bool TryCreateDirectories(const boost::filesystem::path& p)
 620 |  {
 621 |      try
 622 |      {
 623 | -        return boost::filesystem::create_directory(p);
 624 | +        return boost::filesystem::create_directories(p);
```
laanwj commented at 2:17 PM on March 3, 2017:

Is this utility function needed at all? I think the definition of fs::create_directories is already exactly what we want here: http://www.boost.org/doc/libs/1_61_0/libs/filesystem/doc/reference.html#create_directories

Returns: true if a new directory was created, otherwise false. Throws: As specified in Error reporting.
benma commented at 11:47 PM on March 6, 2017:

The function is apparently needed, the comment explains why:

/** * Ignores exceptions thrown by Boost's create_directory if the requested directory exists. * Specifically handles case where path p exists, but it wasn't possible for the user to * write to the parent directory. */

While I can't recreate this condition, digging up the issue that introduced this talks about some edge cases where it does happen (Truecrypt, ...?): #432

So we should probably leave it.
laanwj commented at 8:21 AM on March 7, 2017:

Ugh. I think not being able to rely on the postcondition is incredibly ugly, and should simply be patched in boost upstream. But okay...
fanquake commented at 1:01 PM on May 18, 2017: member

This needs a rebase.
benma force-pushed on May 22, 2017
benma commented at 6:41 AM on May 22, 2017: none

Rebased.
benma commented at 8:13 AM on May 27, 2017: none

@dcousens @laanwj could you please re-review (no changes, just a rebase) and merge?
ryanofsky commented at 2:45 PM on June 1, 2017: member

utACK e94c40c770a5de6c95a8c53cfb1717a35c008d83, nice change.
benma commented at 2:18 PM on June 10, 2017: none

@laanwj bump :)
sipa commented at 11:31 PM on June 12, 2017: member

utACK e94c40c770a5de6c95a8c53cfb1717a35c008d83
MarcoFalke commented at 10:35 AM on June 13, 2017: member

utACK e94c40c
in src/util.cpp:656 in e94c40c770 outdated
```
 652 | @@ -653,15 +653,15 @@ bool RenameOver(fs::path src, fs::path dest)
 653 |  }
 654 |  
 655 |  /**
 656 | - * Ignores exceptions thrown by Boost's create_directory if the requested directory exists.
 657 | + * Ignores exceptions thrown by Boost's create_directories if the requested directory exists.
```
flack commented at 5:22 PM on June 13, 2017:

Since you're touching this line anyways, you could also remove the reference to Boost, which isn't used here anymore (also, the comment at the end of the function could be updated)

benma commented at 10:04 PM on June 13, 2017:

But it is using Boost.

namespace fs = boost::filesystem;.

I fixed the comment at the end of the function.

flack commented at 12:05 PM on June 14, 2017:

hm, ok. I was under the impression that this was changed in https://github.com/benma/bitcoin/commit/bac5c9cf643e9333479ac667426d0b70f8f3aa7f, but I guess I misread

Turn TryCreateDirectory() into TryCreateDirectories()

Use case: TryCreateDirectory(GetDataDir() / "blocks" / "index") would
fail if the blocks directory was not explicitly created before.

The line that did so was in a weird location and could be removed as a
result.

1d1ea9f096

benma force-pushed on Jun 13, 2017
laanwj merged this on Jun 14, 2017
laanwj closed this on Jun 14, 2017
laanwj referenced this in commit 228c319a94 on Jun 14, 2017
PastaPastaPasta referenced this in commit 42c8e2dc27 on Jul 5, 2019
PastaPastaPasta referenced this in commit 0192abf998 on Jul 5, 2019
PastaPastaPasta referenced this in commit 40543e7c12 on Jul 6, 2019
PastaPastaPasta referenced this in commit 51d340537a on Jul 8, 2019
PastaPastaPasta referenced this in commit 8189918e16 on Jul 9, 2019
PastaPastaPasta referenced this in commit c520de2396 on Jul 9, 2019
barrystyle referenced this in commit b21eafa98f on Jan 22, 2020
random-zebra referenced this in commit edfaec63c2 on May 1, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels