Re: [PATCH v2 02/10] Define utility functions for object IDs.
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Mar 11, 2015, 12:44 UTC
- Message-ID
- <550038C8.80601@alum.mit.edu>
- In-Reply-To
- <1425770645-628957-3-git-send-email-sandals@crustytoothpaste.net>
On 03/08/2015 12:23 AM, brian m. carlson wrote:
Show 40 quoted lines
> There are several utility functions (hashcmp and friends) that are used
> for comparing object IDs (SHA-1 values). Using these functions, which
> take pointers to unsigned char, with struct object_id requires tiresome
> access to the sha1 member, which bloats code and violates the desired
> encapsulation. Provide wrappers around these functions for struct
> object_id for neater, more maintainable code. Use the new constants to
> avoid the hard-coded 20s and 40s throughout the original functions.
>
> These functions simply call the underlying pointer-to-unsigned-char
> versions to ensure that any performance improvements will be passed
> through to the new functions.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> I'm not very excited about having to put the #include in the middle of
> cache.h. The alternative, of course, is to move enum object_type up,
> which I can do if necessary. Another alternative is to simply move the
> struct object_id definitions to cache.h instead of object.h, which might
> be cleaner.
>
> I'm interested in hearing opinions about the best way to go forward.
>
> cache.h | 37 +++++++++++++++++++++++++++++++++----
> hex.c | 16 +++++++++++++---
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 761c570..f9addcc 100644
> --- a/cache.h
> +++ b/cache.h
> [...]
> @@ -724,20 +729,42 @@ static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
> return 0;
> }
>
> +static inline int oidcmp(const struct object_id *o1, const struct object_id *o2)
> +{
> + return hashcmp(o1->sha1, o2->sha1);
> +}
> +Maybe rename o1 -> oid1 and o2 -> oid2 just to make things blindingly obvious?
> [...]
Michael
-- Michael Haggerty mhagger@alum.mit.edu