Skip to content

Commit

Permalink
Changed return value of git_oid_match to be consistent with the other…
Browse files Browse the repository at this point in the history
… compare methods (0 means oids match). Added method to compare prefixes of hex formatted oids.
  • Loading branch information
pegonma authored and vmg committed Jun 1, 2011
1 parent ac2b94a commit da03c9f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
2 changes: 1 addition & 1 deletion include/git2/oid.h
Expand Up @@ -142,7 +142,7 @@ GIT_EXTERN(int) git_oid_cmp(const git_oid *a, const git_oid *b);
* @param len the number of hex chars to compare
* @param a first oid structure.
* @param b second oid structure.
* @return 1 in case of a match
* @return 0 in case of a match
*/
GIT_EXTERN(int) gid_oid_match(unsigned int len, git_oid *a, git_oid *b);

Expand Down
11 changes: 8 additions & 3 deletions src/oid.c
Expand Up @@ -177,15 +177,20 @@ int git_oid_match_raw(unsigned int len, const unsigned char *a, const unsigned c
{
do {
if (*a != *b)
return 0;
return 1;
a++;
b++;
len -= 2;
} while (len > 1);
if (len)
if ((*a ^ *b) & 0xf0)
return 0;
return 1;
return 1;
return 0;
}

int git_oid_match_hex(unsigned int len, const unsigned char *a, const unsigned char *b)
{
return memcmp(a, b, len);
}

int gid_oid_match(unsigned int len, git_oid *a, git_oid *b)
Expand Down
12 changes: 11 additions & 1 deletion src/oid.h
@@ -1,7 +1,17 @@
#ifndef INCLUDE_oid_h__
#define INCLUDE_oid_h__

/* This can be useful for internal use */
/**
* Compare the first ('len'*4) bits of two raw formatted oids.
* This can be useful for internal use.
* Return 0 if they match.
*/
int git_oid_match_raw(unsigned int len, const unsigned char *a, const unsigned char *b);

/**
* Compare the first 'len' characters of two hex formatted oids.
* Return 0 if they match.
*/
int git_oid_match_hex(unsigned int len, const unsigned char *a, const unsigned char *b);

#endif

4 comments on commit da03c9f

@carlosmn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this. You apparently feel differently, but for me a function with match in it means "does it match?" so I expect a "true" response if they do. A compare, on the other hand, means "what's the difference?" so I expect a zero if the difference is none, i.e. they are the same.

Furthermore, why is there a git_oid_match and a git_oid_cmp if they are pretty much the same?

@pegonma
Copy link
Contributor Author

@pegonma pegonma commented on da03c9f Jun 2, 2011 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Uncommon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning 0 in case of a match fits the model set by standard functions like strcmp and memcmp, so this makes sense to me.

@carlosmn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pegonma fair enough, there is indeed a point to the extra function, mea culpa for not spotting that, but then IMO a better name would be git_oid_ncmp so it fits in better, as it does the same as git_oid_cmp but only up to a certain amount of chars.

Please sign in to comment.