You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2013/07/01 14:01:15 UTC
Re: svn commit: r1498169 - in
/subversion/branches/fsfs-format7/subversion/libsvn_fs_x: fs_x.c
hotcopy.c pack.c recovery.c revprops.c revprops.h
stefan2@apache.org wrote on Sun, Jun 30, 2013 at 18:46:54 -0000:
> Author: stefan2
> Date: Sun Jun 30 18:46:53 2013
> New Revision: 1498169
>
> URL: http://svn.apache.org/r1498169
> Log:
> On the fsfs-format7 branch: fix a linker issue with the new fsx backend.
> Give all non-static function a 'svn_fs_x__' prefix. Update callers
>
I think that's a "workaround", not a "fix".
The problem was that some functions were declared (and defined) in both
libsvn_fs_x and libsvn_fs_fs, with the same name, same signature, and
without a 'static' modifier in either case (since those functions were
intended to be library-scope).
Your change *avoids* the problem, but it doesn't *fix* it: if we have in
the future another instance of identically-declared function in two FS
backends, the situation where libsvn_fs_x calls into the libsvn_fs_fs
version of the function may repeat. (That'll hopefully result in
a quick error or segfault.)
It seems to me looking into the linker options --- and seeing how
libsvn_fs_x.so code managed to call into libsvn_fs_fs.so code --- would
still be an interesting exercise.
Cheers,
Daniel
> * subversion/libsvn_fs_x/revprops.h
> (write_revprop_generation_file,
> cleanup_revprop_namespace,
> upgrade_pack_revprops,
> upgrade_cleanup_pack_revprops,
> get_revision_proplist,
> set_revision_proplist,
> packed_revprop_available,
> copy_revprops,
> pack_revprops_shard,
> delete_revprops_shard): rename these to ...
> (svn_fs_x__write_revprop_generation_file,
> svn_fs_x__cleanup_revprop_namespace,
> svn_fs_x__upgrade_pack_revprops,
> svn_fs_x__upgrade_cleanup_pack_revprops,
> svn_fs_x__get_revision_proplist,
> svn_fs_x__set_revision_proplist,
> svn_fs_x__packed_revprop_available,
> svn_fs_x__copy_revprops,
> svn_fs_x__pack_revprops_shard,
> svn_fs_x__delete_revprops_shard): ... these
>
> * subversion/libsvn_fs_x/revprops.c
> (): same renames as above
> (svn_fs_x__upgrade_pack_revprops,
> svn_fs_x__upgrade_cleanup_pack_revprops,
> end_revprop_change,
> svn_fs_x__pack_revprops_shard): update callers
>
> * subversion/libsvn_fs_x/fs_x.c
> (upgrade_body,
> svn_fs_x__revision_proplist,
> write_revision_zero,
> change_rev_prop_body): ditto
>
> * subversion/libsvn_fs_x/hotcopy.c
> (hotcopy_body): same here
>
> * subversion/libsvn_fs_x/pack.c
> (pack_shard): and here
>
> * subversion/libsvn_fs_x/recovery.c
> (recover_body): and here
>
> Modified:
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/fs_x.c
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/hotcopy.c
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/pack.c
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/recovery.c
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/revprops.c
> subversion/branches/fsfs-format7/subversion/libsvn_fs_x/revprops.h
>
> Modified: subversion/branches/fsfs-format7/subversion/libsvn_fs_x/fs_x.c
> URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-format7/subversion/libsvn_fs_x/fs_x.c?rev=1498169&r1=1498168&r2=1498169&view=diff
> ==============================================================================
> --- subversion/branches/fsfs-format7/subversion/libsvn_fs_x/fs_x.c (original)
> +++ subversion/branches/fsfs-format7/subversion/libsvn_fs_x/fs_x.c Sun Jun 30 18:46:53 2013
> @@ -683,12 +683,12 @@ upgrade_body(void *baton, apr_pool_t *po
> && max_files_per_dir > 0)
> {
> needs_revprop_shard_cleanup = TRUE;
> - SVN_ERR(upgrade_pack_revprops(fs,
> - upgrade_baton->notify_func,
> - upgrade_baton->notify_baton,
> - upgrade_baton->cancel_func,
> - upgrade_baton->cancel_baton,
> - pool));
> + SVN_ERR(svn_fs_x__upgrade_pack_revprops(fs,
> + upgrade_baton->notify_func,
> + upgrade_baton->notify_baton,
> + upgrade_baton->cancel_func,
> + upgrade_baton->cancel_baton,
> + pool));
> }
>
> /* Bump the format file. */
Re: svn commit: r1498169 - in /subversion/branches/fsfs-format7/subversion/libsvn_fs_x:
fs_x.c hotcopy.c pack.c recovery.c revprops.c revprops.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Jul 1, 2013 at 2:01 PM, Daniel Shahaf <da...@elego.de> wrote:
> stefan2@apache.org wrote on Sun, Jun 30, 2013 at 18:46:54 -0000:
> > Author: stefan2
> > Date: Sun Jun 30 18:46:53 2013
> > New Revision: 1498169
> >
> > URL: http://svn.apache.org/r1498169
> > Log:
> > On the fsfs-format7 branch: fix a linker issue with the new fsx backend.
> > Give all non-static function a 'svn_fs_x__' prefix. Update callers
> >
>
> I think that's a "workaround", not a "fix".
>
> The problem was that some functions were declared (and defined) in both
> libsvn_fs_x and libsvn_fs_fs, with the same name, same signature, and
> without a 'static' modifier in either case (since those functions were
> intended to be library-scope).
>
> Your change *avoids* the problem, but it doesn't *fix* it: if we have in
> the future another instance of identically-declared function in two FS
> backends, the situation where libsvn_fs_x calls into the libsvn_fs_fs
> version of the function may repeat. (That'll hopefully result in
> a quick error or segfault.)
>
Well, the "bug" was that non-static function names should be
prefixed with the respective module name but these were not.
With "fixed" I meant: "now they are".
It seems to me looking into the linker options --- and seeing how
> libsvn_fs_x.so code managed to call into libsvn_fs_fs.so code --- would
> still be an interesting exercise.
>
Yes, that would probably be insightful.
-- Stefan^2.
Re: svn commit: r1498169 - in /subversion/branches/fsfs-format7/subversion/libsvn_fs_x:
fs_x.c hotcopy.c pack.c recovery.c revprops.c revprops.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Jul 1, 2013 at 2:01 PM, Daniel Shahaf <da...@elego.de> wrote:
> stefan2@apache.org wrote on Sun, Jun 30, 2013 at 18:46:54 -0000:
> > Author: stefan2
> > Date: Sun Jun 30 18:46:53 2013
> > New Revision: 1498169
> >
> > URL: http://svn.apache.org/r1498169
> > Log:
> > On the fsfs-format7 branch: fix a linker issue with the new fsx backend.
> > Give all non-static function a 'svn_fs_x__' prefix. Update callers
> >
>
> I think that's a "workaround", not a "fix".
>
> The problem was that some functions were declared (and defined) in both
> libsvn_fs_x and libsvn_fs_fs, with the same name, same signature, and
> without a 'static' modifier in either case (since those functions were
> intended to be library-scope).
>
> Your change *avoids* the problem, but it doesn't *fix* it: if we have in
> the future another instance of identically-declared function in two FS
> backends, the situation where libsvn_fs_x calls into the libsvn_fs_fs
> version of the function may repeat. (That'll hopefully result in
> a quick error or segfault.)
>
Well, the "bug" was that non-static function names should be
prefixed with the respective module name but these were not.
With "fixed" I meant: "now they are".
It seems to me looking into the linker options --- and seeing how
> libsvn_fs_x.so code managed to call into libsvn_fs_fs.so code --- would
> still be an interesting exercise.
>
Yes, that would probably be insightful.
-- Stefan^2.