Add support for deleting directories with -a -E

przemoc / metastore

Store and restore metadata from a filesystem.

http://software.przemoc.net/#metastore

GNU General Public License v2.0

169 stars 31 forks source link

Add support for deleting directories with -a -E #15

Closed dfandrich closed 8 years ago

dfandrich commented 9 years ago

github's default title leave too much to the imagination. This should have been titled Add support for deleting directories with -a -E. This fixes (#10).

przemoc commented 9 years ago

Thanks, looks fine at very quick first glance. I'll check it more thoroughly some other day.

BTW Please make your future topic branches derived directly from origin/master, unless it's truly necessary to base them on some other topic branch - here it is definitely not. (If you want to check code from many branches together you can always create your own local copy branch and rebase it on top of other one and so on...)

dfandrich commented 9 years ago

I combined this branch one with the DEBUG one since you said you'd be merging it, and this way it avoids an annoying merge commit.

przemoc commented 9 years ago

I have to say (finally, after so many months, sorry!) that I don't like this simpleminded algorithm. Being simple isn't necessarily wrong, but it is too naive and quite expensive, especially for bigger sets of nested directories as you already commented in the code (I haven't performed the tests yet to see how it actually behaves in practice).

Other (minor) nitpicks are:

maintaining extradirs even when they're not used later,
wrong white space at the beginning of fixup_newemptydirs() (4 spaces instead of tab),
preincrementing integer instead of postincrementing it when it is the only thing in expression statement. (I use pre-inc/dec only for non-POD in C++, of course unless post- behavior is desired in given expression.)

dfandrich commented 9 years ago

The algorithm may not win any contests from an algorithmic point of view, but it's a giant win for simplicity, and remains completely acceptable from a practical standpoint. For example, I extracted the Linux 3.19 tree, removed the files and ran metastore to delete all 3122 directories and it took 154 ms.

I've dealt with your nitpicks except for the one about maintaining extradirs, which I'm not sure about. extradirs is handled exactly the same way as missingdirs and missingothers. If you're talking about freeing the structure before exit, I don't see the point. It would be just wasting cycles compared to letting the OS free the memory in one go at process exit time. And you can't do something like memory leak detection anyway unless every such instance is changed.

przemoc commented 8 years ago

Fair enough. Applied with small corrections in b6f60de37d17 and improved in further commits. Thanks.