{"thread":{"id":"36562","subject":"[PATCH] Define constants for lengths of object names","startedAt":"2014-05-01T11:06:50Z","lastAt":"2014-05-02T22:09:08Z","messageCount":7,"participants":["brian m. carlson","Jonathan Nieder","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"240400","messageId":"1398942410-112069-1-git-send-email-sandals@crustytoothpaste.net","threadId":"36562","inReplyTo":null,"subject":"[PATCH] Define constants for lengths of object names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-01T11:06:50Z","receivedAt":"2014-05-01T11:06:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Using preprocessor constants rather than hardcoded numbers is considered a\ngood programming practice.  Provide two constants, GIT_OID_RAWSZ and\nGIT_OID_HEXSZ, which are the lengths of an SHA-1 object name in bytes and\nhex digits, respectively.  These names are the same as those used by\nlibgit2.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n object.h | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/object.h b/object.h\nindex 6e12f2c..f1cff2d 100644\n--- a/object.h\n+++ b/object.h\n@@ -1,6 +1,13 @@\n #ifndef OBJECT_H\n #define OBJECT_H\n \n+/*\n+ * The length in bytes and in hex digits of an object name (SHA-1 value).\n+ * These are the same names used by libgit2.\n+ */\n+#define GIT_OID_RAWSZ 20\n+#define GIT_OID_HEXSZ 40\n+\n struct object_list {\n \tstruct object *item;\n \tstruct object_list *next;\n@@ -49,7 +56,7 @@ struct object {\n \tunsigned used : 1;\n \tunsigned type : TYPE_BITS;\n \tunsigned flags : FLAG_BITS;\n-\tunsigned char sha1[20];\n+\tunsigned char sha1[GIT_OID_RAWSZ];\n };\n \n extern const char *typename(unsigned int type);\n-- \n2.0.0.rc0\n"},{"id":"240407","messageId":"20140501172007.GZ9218@google.com","threadId":"36562","inReplyTo":"1398942410-112069-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-01T17:20:07Z","receivedAt":"2014-05-01T17:20:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nbrian m. carlson wrote:\n\n> --- a/object.h\n> +++ b/object.h\n[...]\n> @@ -49,7 +56,7 @@ struct object {\n>  \tunsigned used : 1;\n>  \tunsigned type : TYPE_BITS;\n>  \tunsigned flags : FLAG_BITS;\n> -\tunsigned char sha1[20];\n> +\tunsigned char sha1[GIT_OID_RAWSZ];\n\nMaybe my brain has been damaged by reading code from too many C\nprojects that hard-code some constants, but I find '20' easier to read\nhere.\n\nWhat happened to the\n\n\tstruct object_id {\n\t\tunsigned char id[20];\n\t};\n\n\t...\n\n\tstruct object {\n\t\t...\n\t\tstruct object_id id;\n\t};\n\nidea?\n\nThanks,\nJonathan\n"},{"id":"240414","messageId":"20140501182304.GB9218@google.com","threadId":"36562","inReplyTo":"20140501172007.GZ9218@google.com","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-01T18:23:04Z","receivedAt":"2014-05-01T18:23:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> brian m. carlson wrote:\n\n>> --- a/object.h\n>> +++ b/object.h\n> [...]\n>> @@ -49,7 +56,7 @@ struct object {\n>>  \tunsigned used : 1;\n>>  \tunsigned type : TYPE_BITS;\n>>  \tunsigned flags : FLAG_BITS;\n>> -\tunsigned char sha1[20];\n>> +\tunsigned char sha1[GIT_OID_RAWSZ];\n>\n> Maybe my brain has been damaged by reading code from too many C\n> projects that hard-code some constants, but I find '20' easier to read\n> here.\n\nThat said, RAW_SHA1_BYTES would sound okay to me.\n\nPart of the problem was how long it takes to mentally parse oid_rawsz.\n\nThanks,\nJonathan\n"},{"id":"240482","messageId":"20140501230041.GC75770@vauxhall.crustytoothpaste.net","threadId":"36562","inReplyTo":"20140501172007.GZ9218@google.com","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-01T23:00:41Z","receivedAt":"2014-05-01T23:00:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote:\n> Hi,\n> \n> brian m. carlson wrote:\n> \n> > --- a/object.h\n> > +++ b/object.h\n> [...]\n> > @@ -49,7 +56,7 @@ struct object {\n> >  \tunsigned used : 1;\n> >  \tunsigned type : TYPE_BITS;\n> >  \tunsigned flags : FLAG_BITS;\n> > -\tunsigned char sha1[20];\n> > +\tunsigned char sha1[GIT_OID_RAWSZ];\n> \n> Maybe my brain has been damaged by reading code from too many C\n> projects that hard-code some constants, but I find '20' easier to read\n> here.\n> \n> What happened to the\n> \n> \tstruct object_id {\n> \t\tunsigned char id[20];\n> \t};\n> \n> \t...\n> \n> \tstruct object {\n> \t\t...\n> \t\tstruct object_id id;\n> \t};\n> \n> idea?\n\nThere didn't seem to be a huge amount of support for it.  Also, there\nwere concerns that some architectures might impose alignment constraints\non it that made sizeof(struct object_id) != 20.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"240483","messageId":"20140501230553.GE9218@google.com","threadId":"36562","inReplyTo":"20140501230041.GC75770@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-01T23:05:53Z","receivedAt":"2014-05-01T23:05:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"brian m. carlson wrote:\n> On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote:\n\n>> What happened to the\n>>\n>> \tstruct object_id {\n>> \t\tunsigned char id[20];\n>> \t};\n>>\n>> \t...\n>>\n>> \tstruct object {\n>> \t\t...\n>> \t\tstruct object_id id;\n>> \t};\n>>\n>> idea?\n>\n> There didn't seem to be a huge amount of support for it.\n\nI can make up for it in enthuasiasm.  Please?  It's something I've\nwanted for a long time but never found the time to do.\n\n>                                                           Also, there\n> were concerns that some architectures might impose alignment constraints\n> on it that made sizeof(struct object_id) != 20.\n\nSounds awful.  What architecture?\n\nThanks,\nJonathan\n"},{"id":"240500","messageId":"CACsJy8AhxJDcBHNWbssWs2F1oQB4xGG7rpBePX60m4=_+MVEZw@mail.gmail.com","threadId":"36562","inReplyTo":"20140501230553.GE9218@google.com","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-02T00:15:44Z","receivedAt":"2014-05-02T00:15:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, May 2, 2014 at 6:05 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> brian m. carlson wrote:\n>> On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote:\n>\n>>> What happened to the\n>>>\n>>>      struct object_id {\n>>>              unsigned char id[20];\n>>>      };\n>>>\n>>>      ...\n>>>\n>>>      struct object {\n>>>              ...\n>>>              struct object_id id;\n>>>      };\n>>>\n>>> idea?\n>>\n>> There didn't seem to be a huge amount of support for it.\n>\n> I can make up for it in enthuasiasm.  Please?  It's something I've\n> wanted for a long time but never found the time to do.\n\nIt's definitely better in the sense that the compiler will catch new\n\"char[20]\" declarations for us. It's also a lot more work.\n\n>>                                                           Also, there\n>> were concerns that some architectures might impose alignment constraints\n>> on it that made sizeof(struct object_id) != 20.\n>\n> Sounds awful.  What architecture?\n\nNo architecture was named last time if I remember correctly. But we\ncould check \"sizeof(struct object_id) == 20\" in a test or something.\nWhen people scream, we can pack the struct on that particular\nplatform?\n-- \nDuy\n"},{"id":"240582","messageId":"20140502220908.GI75770@vauxhall.crustytoothpaste.net","threadId":"36562","inReplyTo":"CACsJy8AhxJDcBHNWbssWs2F1oQB4xGG7rpBePX60m4=_+MVEZw@mail.gmail.com","subject":"Re: [PATCH] Define constants for lengths of object names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-02T22:09:08Z","receivedAt":"2014-05-02T22:09:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, May 02, 2014 at 07:15:44AM +0700, Duy Nguyen wrote:\n> On Fri, May 2, 2014 at 6:05 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> >\n> > I can make up for it in enthuasiasm.  Please?  It's something I've\n> > wanted for a long time but never found the time to do.\n> \n> It's definitely better in the sense that the compiler will catch new\n> \"char[20]\" declarations for us. It's also a lot more work.\n\nIt is.  I'm going to start with a patch that introduces struct object_id\nand the fixed constants.  Then I'm going to get a patch that compiles\nwith lots of warnings, and then I'm going to fix all those warnings.\nOtherwise, the patch will simply be too enormous to review.\n\nI'm willing to hear other suggestions for going about this, though.\n\n> No architecture was named last time if I remember correctly. But we\n> could check \"sizeof(struct object_id) == 20\" in a test or something.\n> When people scream, we can pack the struct on that particular\n> platform?\n\nSounds like a plan.  I am not aware of any architecture that has this\nlimitation; I've worked with x86(-64)?, 32-bit PowerPC, UltraSPARC, and\nARM.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"}]}