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.
Please let me know how to change trunk.
Thanks,
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...)
>
>
