Re: Typesafer git hash patch
From: brian m. carlson <hidden>
Date: 2017-02-28 20:57:54
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 [off-list ref] wrote:quoted
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
Attachments
- signature.asc [application/pgp-signature] 868 bytes