Daniel Shahaf wrote:
> The backport proposal now includes a fix of the original callsite
> (r1146708).
>
> Thus, 1.7.x the semantics of these API's are unchanged and the backport
> entry for the problematic caller has been patched. For 1.8.x, given
> that I'm juiced out by this thread, I'll go ahead and implement on trunk
> whatever consensus@ decides.
>
> I have observed that all users of _display() (except one in
> adm-crawler.c, where the exception to the rule is questioned by a ###
> comment) use the _display() API either for error messages or for
> stringifying the digest into constructing paths and other objects.
>
> I believe that error messages don't distinguish between a NULL return
> and a "(null)" return, and that path construction, etc, would probably
> work in a bogus manner if NULL is returned (think svn_dirent_join_many())
> and work-and-maybe-fail-later if "(null)" is returned.
Thanks for checking. So the function is being used for two different
purposes, one of which is what its name implies (where accepting null
input could be useful), and the other is not (and logically requires a
non-null input).
> Please let me know how to change trunk.
My advice is to revert it to how it was (input must be non-null).
- Julian
> Julian Foad wrote on Thu, Jul 14, 2011 at 11:45:46 +0100:
> > Daniel Shahaf wrote:
> > > Oh. And I realized that svn_checksum_to_cstring() only started to
> > > accept NULL in 1.7. Which means that, currently, in 1.7.x,
> > >
> > > * svn_checksum_to_cstring(NULL) returns NULL
> > > * svn_checksum_to_cstring_display(NULL) segfaults
> >
> > The essential and original difference between the two functions is that
> > given an all-zero input, the former returns NULL whereas the latter
> > returns a printable representation ("0000000000000000000000...").
> >
> > So it seems to me that if the _display function is going to do anything
> > with a null input, it should return some printable representation such
> > as "(null)", and never return a null pointer.
> >
> > Alternatively I think it's fine to leave it the way it was, requiring a
> > non-null input, even though the other function accepts a null input.
> >
> > - Julian
> >
> >
> > >
> > > Generally I prefer consistency before I prefer that My Way gets
> > > chosen... which means that I would prefer that these two API's behave
> > > the same way in 1.7. (In 1.6 both of them segfault.)
> > >
> > > Daniel
> > > (I don't have /too/ much energy left for this discussion...)
Received on 2011-07-14 18:14:59 CEST