Thread (2 messages) 2 messages, 2 authors, 2017-02-28

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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help