On Mon, 2008-12-15 at 08:49 -0600, Hyrum K. Wright wrote:
> Kouhei Sutou wrote:
> > Hi,
> >
> > I don't know about svn_fs_fs__noderev_same_rep_key() but it
> > causes "Segmentation fault" if a->uniquifier or
> > b->uniquifier is NULL. Should it accepts NULL or not?
>
> Yes. When reading pre-format-4 versions of the file system, the uniquifier will
> be NULL, so we should account for this possibility.
Can you make the doc string for the "uniquifier" field indicate that,
then, please? Presently it doesn't indicate that the field may be null.
> > If it accepts NULL as valid VALUE:
> >
> > 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)
> > @@ -3435,6 +3435,12 @@
> > if (a->revision != b->revision)
> > return FALSE;
> >
> > + if (a->uniquifier == b->uniquifier)
>
> Should we explicitly make sure that these are both equal to NULL?
The comparison a few lines further up of the whole representation_t uses
this form, which allows for the fact that if the pointers are equal, the
objects pointed to are definitely equal.
> > + 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)
)
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=984500
Received on 2008-12-15 16:07:45 CET