[PATCH] Remove unused functions [was: Sharing libsvn_fs_* internals]

[Julian Foad <ju...@btopenworld.com>]

Greg Hudson wrote:
> On Wed, 2005-11-02 at 13:33 +0000, Julian Foad wrote:
> 
>>To recap, I'm talking about the two files subversion/libsvn_fs_{base,fs}/err.c 
>>being almost identical.
> 
> Note that most of the functions in libsvn_fs_fs/err.c are not used.

Good grief - so they're not.  And a few in libsvn_fs_base/err.c too.

The attached patch removes them.

> Prior to the addition of lock.c, there were only four uses of the error
> helpers in all of FSFS.
> 
> Also note that having error helper functions degrades the utility of
> file and line numbers in the --enable-maintainer-mode messages, and is
> not a practice we use widely outside the FS code.

Yes - I thought it was a bit odd, but I'm not going to be the one to change it.


>>It's not just those files.  Presumably, if the two filesystems use the same set 
>>of error messages, then there is much in common in their implementation.  I 
>>haven't looked further in detail.
> 
> Although the FSFS code is based off the BDB code, I don't think there's
> much code to share (there's a path canonicalization function and some
> key generation functions, which shouldn't exist).  Much of the shared

Good grief - stuff that "shouldn't exist"!  The attached patch removes the 
completely unused key-generation functions, but doesn't address those that are 
still being used but shouldn't be.

> logic is at the higher levels ("how to open a path"), but calls out to
> different logic at the lower levels ("how to open a node-rev and read
> directory entries").  It's possible to share higher-level logic with
> lower-level differences, but it tends to be more awkward than sharing
> lower-level logic.
> 
> Also, there's a fair amount of cleanup which could be done to the FSFS
> code which would render a lot of the BDB code vestigial.  For instance,
> the dag layer doesn't really do much in FSFS, although eliminating it
> would take some doing.

Well, it would be worth cleaning up the FSFS code now that it's intended to be 
widely used and supported.  Any volunteers?


>>At the moment, while I'd be happy to go the clean, layered, potentially public 
>>route, I don't think the amount of sharing is large enough to require that, and 
>>so I'd be happy to put more shared stuff in libsvn_fs which is simpler in the 
>>short term, as long as we keep our eyes open for the point when it starts to 
>>get unreasonably complex and do the refactoring then.
> 
> We actually did put small amounts of shared code into libsvn_fs
> initially, but ran into problems on OSX and Windows.

Thanks for these words of wisdom and experience, Greg.

- Julian

Re: [PATCH] Remove unused functions [was: Sharing libsvn_fs_* internals]

[Greg Hudson <gh...@MIT.EDU>]

On Thu, 2005-11-03 at 01:45 +0000, Julian Foad wrote:
> > Although the FSFS code is based off the BDB code, I don't think there's
> > much code to share (there's a path canonicalization function and some
> > key generation functions, which shouldn't exist).  Much of the shared
> 
> Good grief - stuff that "shouldn't exist"!

Sorry, I have no idea why I wrote "which shouldn't exist" in that
parenthetical.  The key generation functions should exist as far as I
know.  I think I meant to say that those don't amount to much code, and
my brain wandered.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org