Julian Foad wrote:
> On Mon, 2008-12-15 at 08:49 -0600, Hyrum K. Wright wrote:
>> Kouhei Sutou wrote:
>>> 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.
Done in r34720.
>>> 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)
Could you commit the changes to fs_fs.c?
Received on 2008-12-15 19:26:01 CET