From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>,
"brian m. carlson" <sandals@crustytoothpaste.net>
Cc: "Kyle J. McKay" <mackyle@gmail.com>,
git@vger.kernel.org, Andreas Schwab <schwab@linux-m68k.org>
Subject: Re: [PATCH v2 01/10] Define a structure for object IDs.
Date: Thu, 12 Mar 2015 11:28:10 +0100 [thread overview]
Message-ID: <55016A3A.6010100@alum.mit.edu> (raw)
In-Reply-To: <xmqqr3sv3vsf.fsf@gitster.dls.corp.google.com>
On 03/12/2015 01:26 AM, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> Michael Haggerty recommended that I call the structure element sha1
>> instead of oid in case we want to turn this into a union if we decide to
>> go the additional hash route.
>
> I'd advise against it.
>
> As I wrote in $gmane/265337 in response to Michael:
>
> > 4. We continue to support working with SHA-1s declared to be (unsigned
> > char *) in some performance-critical code, even as we migrate most other
> > code to using SHA-1s embedded within a (struct object_id). This will
> > cost some duplication of code. To accept this approach, we would need an
> > idea of *how much* code duplication would be needed. E.g., how many
> > functions will need both (unsigned char *) versions and (struct
> > object_id *) versions?
>
> ...
>
> I do not know what kind of code duplication you are worried about,
> though. If a callee needs "unsigned char *", the caller that has a
> "struct object_id *o" should pass o->hash to the callee.
>
> And that would break the abstraction effort if you start calling the
> field with a name that is specific to the underlying hash function.
> The caller has to change o->sha1 to o->sha256 instead of keeping
> that as o->oid and letting the callee handle the implementation
> details when calling
>
> if (!hashcmp(o1->oid, o2->oid))
> ; /* they are the same */
> else
> ; /* they are different */
> [...]
Hmm, I guess you imagine that we might sometimes pack SHA-1s, sometimes
SHA-256s (or whatever) in the "oid" field, which would be dimensioned
large enough for either one (with, say, SHA-1s padded with zeros).
I was imagining that this would evolve into a union (or maybe struct) of
different hash types, like
struct object_id {
unsigned char hash_type;
union {
unsigned char sha1[GIT_SHA1_RAWSZ];
unsigned char sha256[GIT_SHA256_RAWSZ];
} hash;
};
BTW in either case, any hopes of mapping object_id objects directly on
top of buffer memory would disappear.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
next prev parent reply other threads:[~2015-03-12 10:28 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-07 23:23 [PATCH v2 00/10] Use a structure for object IDs brian m. carlson
2015-03-07 23:23 ` [PATCH v2 01/10] Define " brian m. carlson
[not found] ` <CEA07500-9F47-4B24-AD5D-1423A601A4DD@gmail.com>
2015-03-11 22:08 ` brian m. carlson
2015-03-12 0:26 ` Junio C Hamano
2015-03-12 9:34 ` brian m. carlson
2015-03-12 10:28 ` Michael Haggerty [this message]
2015-03-12 10:46 ` brian m. carlson
2015-03-12 11:16 ` Duy Nguyen
2015-03-12 18:24 ` Junio C Hamano
2015-03-12 18:44 ` Junio C Hamano
2015-03-13 0:58 ` Duy Nguyen
2015-03-13 6:03 ` Junio C Hamano
2015-03-14 11:49 ` Duy Nguyen
2015-03-14 22:19 ` Junio C Hamano
2015-03-15 0:17 ` Duy Nguyen
2015-03-15 2:32 ` Junio C Hamano
2015-03-07 23:23 ` [PATCH v2 02/10] Define utility functions " brian m. carlson
2015-03-08 9:57 ` Duy Nguyen
2015-03-08 14:48 ` brian m. carlson
2015-03-11 12:44 ` Michael Haggerty
2015-03-07 23:23 ` [PATCH v2 03/10] bisect.c: convert leaf functions to use struct object_id brian m. carlson
2015-03-07 23:23 ` [PATCH v2 04/10] archive.c: convert " brian m. carlson
2015-03-11 14:20 ` Michael Haggerty
2015-03-11 22:12 ` brian m. carlson
2015-03-07 23:24 ` [PATCH v2 05/10] zip: use GIT_SHA1_HEXSZ for trailers brian m. carlson
2015-03-07 23:24 ` [PATCH v2 06/10] bulk-checkin.c: convert to use struct object_id brian m. carlson
2015-03-07 23:24 ` [PATCH v2 07/10] diff: convert struct combine_diff_path to object_id brian m. carlson
2015-03-07 23:24 ` [PATCH v2 08/10] commit: convert parts to struct object_id brian m. carlson
2015-03-11 14:46 ` Michael Haggerty
2015-03-07 23:24 ` [PATCH v2 09/10] patch-id: convert to use " brian m. carlson
2015-03-07 23:24 ` [PATCH v2 10/10] apply: convert threeway_stage to object_id brian m. carlson
2015-03-08 7:43 ` [PATCH v2 00/10] Use a structure for object IDs Junio C Hamano
2015-03-11 2:38 ` Kyle J. McKay
2015-03-11 16:08 ` Michael Haggerty
2015-03-11 20:35 ` Junio C Hamano
2015-03-13 22:45 ` brian m. carlson
-- strict thread matches above, loose matches on Subject: below --
2015-03-13 23:39 brian m. carlson
2015-03-13 23:39 ` [PATCH v2 01/10] Define " brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55016A3A.6010100@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mackyle@gmail.com \
--cc=sandals@crustytoothpaste.net \
--cc=schwab@linux-m68k.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.