Re: [PATCH] Define constants for lengths of object names
On Fri, May 02, 2014 at 07:15:44AM +0700, Duy Nguyen wrote: On Fri, May 2, 2014 at 6:05 AM, Jonathan Nieder jrnie...@gmail.com wrote: I can make up for it in enthuasiasm. Please? It's something I've wanted for a long time but never found the time to do. It's definitely better in the sense that the compiler will catch new char[20] declarations for us. It's also a lot more work. It is. I'm going to start with a patch that introduces struct object_id and the fixed constants. Then I'm going to get a patch that compiles with lots of warnings, and then I'm going to fix all those warnings. Otherwise, the patch will simply be too enormous to review. I'm willing to hear other suggestions for going about this, though. No architecture was named last time if I remember correctly. But we could check sizeof(struct object_id) == 20 in a test or something. When people scream, we can pack the struct on that particular platform? Sounds like a plan. I am not aware of any architecture that has this limitation; I've worked with x86(-64)?, 32-bit PowerPC, UltraSPARC, and ARM. -- 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 signature.asc Description: Digital signature
Re: [PATCH] Define constants for lengths of object names
Hi, brian m. carlson wrote: --- a/object.h +++ b/object.h [...] @@ -49,7 +56,7 @@ struct object { unsigned used : 1; unsigned type : TYPE_BITS; unsigned flags : FLAG_BITS; - unsigned char sha1[20]; + unsigned char sha1[GIT_OID_RAWSZ]; Maybe my brain has been damaged by reading code from too many C projects that hard-code some constants, but I find '20' easier to read here. What happened to the struct object_id { unsigned char id[20]; }; ... struct object { ... struct object_id id; }; idea? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Define constants for lengths of object names
Jonathan Nieder wrote: brian m. carlson wrote: --- a/object.h +++ b/object.h [...] @@ -49,7 +56,7 @@ struct object { unsigned used : 1; unsigned type : TYPE_BITS; unsigned flags : FLAG_BITS; -unsigned char sha1[20]; +unsigned char sha1[GIT_OID_RAWSZ]; Maybe my brain has been damaged by reading code from too many C projects that hard-code some constants, but I find '20' easier to read here. That said, RAW_SHA1_BYTES would sound okay to me. Part of the problem was how long it takes to mentally parse oid_rawsz. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Define constants for lengths of object names
On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote: Hi, brian m. carlson wrote: --- a/object.h +++ b/object.h [...] @@ -49,7 +56,7 @@ struct object { unsigned used : 1; unsigned type : TYPE_BITS; unsigned flags : FLAG_BITS; - unsigned char sha1[20]; + unsigned char sha1[GIT_OID_RAWSZ]; Maybe my brain has been damaged by reading code from too many C projects that hard-code some constants, but I find '20' easier to read here. What happened to the struct object_id { unsigned char id[20]; }; ... struct object { ... struct object_id id; }; idea? There didn't seem to be a huge amount of support for it. Also, there were concerns that some architectures might impose alignment constraints on it that made sizeof(struct object_id) != 20. -- 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 signature.asc Description: Digital signature
Re: [PATCH] Define constants for lengths of object names
brian m. carlson wrote: On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote: What happened to the struct object_id { unsigned char id[20]; }; ... struct object { ... struct object_id id; }; idea? There didn't seem to be a huge amount of support for it. I can make up for it in enthuasiasm. Please? It's something I've wanted for a long time but never found the time to do. Also, there were concerns that some architectures might impose alignment constraints on it that made sizeof(struct object_id) != 20. Sounds awful. What architecture? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Define constants for lengths of object names
On Fri, May 2, 2014 at 6:05 AM, Jonathan Nieder jrnie...@gmail.com wrote: brian m. carlson wrote: On Thu, May 01, 2014 at 10:20:07AM -0700, Jonathan Nieder wrote: What happened to the struct object_id { unsigned char id[20]; }; ... struct object { ... struct object_id id; }; idea? There didn't seem to be a huge amount of support for it. I can make up for it in enthuasiasm. Please? It's something I've wanted for a long time but never found the time to do. It's definitely better in the sense that the compiler will catch new char[20] declarations for us. It's also a lot more work. Also, there were concerns that some architectures might impose alignment constraints on it that made sizeof(struct object_id) != 20. Sounds awful. What architecture? No architecture was named last time if I remember correctly. But we could check sizeof(struct object_id) == 20 in a test or something. When people scream, we can pack the struct on that particular platform? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html