Re: Typesafer git hash patch

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

On Tue, Feb 28, 2017 at 12:25:20PM -0800, Linus Torvalds wrote:
> On Tue, Feb 28, 2017 at 11:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Sorry, but at this point in your description, you completely lost
> > me.  I thought "struct object_id" was what you call "hash_t" in the
> > above.
> 
> So what happened was that I started out just encapsulating
> 
>    unsigned char sha1[20];
> 
> as a
> 
>    hash_t hash;
> 
> and that made sense in a lot of situations. I always thought that code that used
> 
>     struct object_id oid;
> 
> is just too ugly to live, so I'm not actually all that big of a fan of
> the oid approach.

There was some discussion on the list about the best name to use, and
object_id seemed like the most popular decision.  I don't care if we add
a typedef for it and prefer that typedef, but the existing code avoided
typedefs in favor of explicit struct definitions.

I'm certainly not opposed to having less to type, because “object_id” is
awkward to type, but I've generally tried to defer to existing uses in
the codebase and what list regulars are comfortable with.

The only problem with using hash_t is that it's then not obvious as we
transition (assuming we don't take an omnibus patch) what is converted
and what isn't.

> But the two approaches really are pretty much equivalent logically,
> even if they don't look the same.

Yeah, I think they are.

> So I wanted to unify things: "One type to bring them all and in the
> darkness bind them".
> 
> So I just basically made this:
> 
>     typedef struct object_id {
>             unsigned char hash[GIT_HASH_SIZE];
>     } hash_t;
> 
> to create one single data structure that doesn't make my eyes bleed.
> That "struct object_id" still exists, but I don't generally have to
> look at it when doing the conversion, and any current users "just
> work".

There is nothing that prevents us from doing a nice global
search-and-replace in the future if we think the status quo is bad.
That's something that could be automated with Coccinelle.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]