[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Should svn_fs_fs__noderev_same_rep_key() accept NULL representation_t::uniquifier?

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 17 Dec 2008 15:02:02 +0000

On Wed, 2008-12-17 at 22:01 +0900, Kouhei Sutou wrote:
> In <4946A121.5030704_at_mail.utexas.edu>
> "Re: Should svn_fs_fs__noderev_same_rep_key() accept NULL representation_t::uniquifier?" on Mon, 15 Dec 2008 12:25:37 -0600,
> "Hyrum K. Wright" <hyrum_wright_at_mail.utexas.edu> wrote:
> > >>> + return TRUE;
> > >>> +
> > >>> + if (a->uniquifier == NULL || b->uniquifier == NULL)
> > >>> + return FALSE;
> > >
> > > (And, for brevity, the comparison of the whole representation_t pointers
> > > 'a' and 'b' could use this form too:
> > >
> > > Index: subversion/libsvn_fs_fs/fs_fs.c
> > > ===================================================================
> > > --- subversion/libsvn_fs_fs/fs_fs.c (revision 34710)
> > > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> > > @@ -3423,10 +3423,7 @@
> > > if (a == b)
> > > return TRUE;
> > >
> > > - if (a && (! b))
> > > - return FALSE;
> > > -
> > > - if (b && (! a))
> > > + if (a == NULL || b == NULL)
> > > return FALSE;
> > >
> > > if (a->offset != b->offset)
> > > )
> >
> > Kou,
> > Could you commit the changes to fs_fs.c?
>
> I've committed the later of my patches in r34759.

[[[
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 34758)
+++ subversion/libsvn_fs_fs/fs_fs.c (revision 34759)
@@ -3432,6 +3432,9 @@
   if (a->revision != b->revision)
     return FALSE;

+ if (a->uniquifier == NULL || b->uniquifier == NULL)
+ return FALSE;
+
   return strcmp(a->uniquifier, b->uniquifier) == 0;
 }
]]]

Kou, did you see the revised definition:

[[[
  /* For rep-sharing, we need a way of uniquifying node-revs which share the
     same representation (see svn_fs_fs__noderev_same_rep_key() ). So, we
     store the original txn of the node rev (not the rep!), along with some
     intra-node uniqification content.

     May be NULL, in which case, it is considered to match other NULL
     values.*/
  const char *uniquifier;
} representation_t;
]]]

Doesn't that make your change wrong?

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=985755
Received on 2008-12-17 16:02:47 CET

This is an archived mail posted to the Subversion Dev mailing list.