On Tue, May 19, 2009 at 7:59 AM, Stefan Sperling <stsp_at_elego.de> wrote:
[snip]
> I'd say a better representation of a wildcard checksum is in order.
> The API currently overloads the meaning of NULL, which I don't think
> is a good idea. Whenever I see a NULL pointer, I associate that with
> "uninitialized" and hence "not OK to use".
Yeah, I'm not typically a fan of that either, although I have to cope
with that sort behavior quite often (mainly from 3rd party libraries)
so it doesn't bother me much.
> I don't really care if it's done by setting pointers to NULL,
> but then we should at least
>
> #define SVN_WILDCARD_CHECKSUM NULL
>
> and have the code say:
>
> if (checksum1 == SVN_WILDCARD_CHECKSUM ||
> checksum2 == SVN_WILDCARD_CHECKSUM)
> return TRUE;
>
> Or going even further, we could add a "wildchard" checksum type.
> Something like this:
> Index: subversion/include/svn_checksum.h
> ===================================================================
> --- subversion/include/svn_checksum.h (revision 37767)
> +++ subversion/include/svn_checksum.h (working copy)
> @@ -43,7 +43,11 @@ typedef enum
> svn_checksum_md5,
>
> /** The checksum is (or should be set to) a SHA1 checksum. */
> - svn_checksum_sha1
> + svn_checksum_sha1,
> +
> + /** The checksum is a wildcard.
> + * @since New in 1.7 */
> + svn_checksum_wildcard
> } svn_checksum_kind_t;
>
[snip]
> svn_checksum_parse_hex() would need a bump of course, but the diff
> above shows just the docstring diff on purpose.
>
> Either approach would make the API more clear I think.
Explicit is better than implicit. :-) FWIW, I think both ideas are
great. I kind of like the second suggestion more, since it officially
makes the wildcard it's own "type".
-John
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2311595
Received on 2009-05-19 14:16:51 CEST