Neels Janosch Hofmeyr wrote:
> Hi all,
>
> I came across this snippet of code:
>
> subversion/libsvn_subr/checksum.c:
> [[[
> svn_boolean_t
> svn_checksum_match(const svn_checksum_t *checksum1,
> const svn_checksum_t *checksum2)
> {
> if (checksum1 == NULL || checksum2 == NULL)
> return TRUE;
> ]]]
>
> I don't see how that makes sense. If it does, then the function name is
> misleading. The comment from include/checksum.h is ... tricky:
I would hazard a guess that it should be returning 'FALSE' and not
'TRUE', which would explain the 'make no sense' part. If it return'd
TRUE, wouldn't something later on choke thinking that the two checksums
equal to each other, when in fact, they don't?
>
> [[[
> /** Compare checksums @a checksum1 and @a checksum2. If their kinds do not
> * match or if neither is all zeros, and their content does not match, then
> * return FALSE; else return TRUE.
> *
> * @since New in 1.6.
> */
> svn_boolean_t
> svn_checksum_match(const svn_checksum_t *checksum1,
> const svn_checksum_t *checksum2);
> ]]]
>
>
> So, which way should I fix this. Like this:
> [[[
> if (checksum1 == NULL || checksum2 == NULL)
> return (checksum1 == checksum2)? TRUE:FALSE;
> ]]]
This means if both were NULL, it will return TRUE.
Otherwise FALSE, right?
Perhaps my understanding of this function isn't to
clear. If I've made any errors in concept, please
correct. I would think if they were both NULL, that
this would turn out to be FALSE; because, the point
of this function is to determine the 'valid sameness'
of the checksums. If either or both checksums are
NULL, then would that not throw the validity out the
window, so to speak? Whatever called this function
has basically supplied two NULL pointers. Sure, they're
the same, but it defeats the purpose of this
function?
> [[[
> /** Compare checksums @a checksum1 and @a checksum2. Return TRUE if both
> * their kinds and content match, FALSE otherwise.
> ]]]
>
>
> Or like this:
> [[[
> /** Compare checksums @a checksum1 and @a checksum2. Return TRUE if both
> * their kinds and content match, FALSE otherwise. However, if one of the
> * checksums is NULL, return TRUE, even if the other one is non-NULL.
> ]]]
That last part would implicate checksum1 != checksum2, which AFAIK,
should still return FALSE.
>
> I am not really getting though what the reference "if neither is all zeros"
> in the original comment is trying to say -- whether it's the NULLness of the
> pointer or whether the checksum digits are all '0'.
I would think the reference is to check if the checksum digits are all
'0's , and not the NULLness of the pointers.
Any corrections in my understanding definitely appreciated.
Edmund
Btw, I don't know if I'm in the right position to say this, but
nice find, Neels.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2307172
Received on 2009-05-19 07:28:27 CEST