Re: [RFC PATCH 0/9] Use a structure for object IDs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, May 04, 2014 at 08:35:00AM +0200, Michael Haggerty wrote:
> On 05/03/2014 10:12 PM, brian m. carlson wrote:
> > I called the structure member "oid" because it was easily grepable and
> > distinct from the rest of the codebase.  It, too, can be changed if we
> > decide on a better name.  I specifically did not choose "sha1" since it
> > looks weird to have "sha1->sha1" and I didn't want to rename lots of
> > variables.
> 
> That means that we will have sha1->oid all over the place, right?
> That's unfortunate, because it is exactly backwards from what we would
> want in a hypothetical future where OIDs are not necessarily SHA-1s.  In
> that future we would certainly have to support SHA-1s in parallel with
> the new hash.  So (in that hypothetical future) we will probably want
> these expressions to look like oid->sha1, to allow, say, a second struct
> or union field oid->sha256 [1].

As Johannes pointed out, only during the transition period.

> If that future would come to pass, then we would also want to have
> distinct constants like GIT_SHA1_RAWSZ and GIT_SHA256_RAWSZ rather than
> the generically-named GIT_OID_RAWSZ.

You have a point.  I'll make the change.

> I think that this patch series will improve the code clarity and type
> safety independent of thoughts about supporting different hash
> algorithms, so I'm not objecting to your naming decision.  But *if* such
> support is part of your long-term hope, then you might ease the future
> transition by choosing different names now.

It is an eventual goal, but without this series, it's not even worth
discussing since it's too hard to implement.  Even if that doesn't
happen, my hope is that we'll at least improve the safety of the code
and hopefully avoid a bug or two out of it.

> (Maybe renaming local variables "sha1 -> oid" might be a handy way of
> making clear which code has been converted to the new style.)

This is a good idea as well.  I'll walk through the patches and fix
that.

> Just to be clear, the above are just some random thoughts for your
> consideration, but feel free to disregard them.

I appreciate the well-thought-out response.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]