On 13/07/14 07:41, Jeff King wrote: > On Sat, Jul 12, 2014 at 02:05:39PM -0400, Jeff King wrote: > >>> I don't particularly like 'flag' here. (not a massive dislike, mind you:) >>> >>> Perhaps: flag->object_type, type->node_type? >>> Or, if that's too verbose, maybe just: flag->type, type->node? >> >> Me either, but as you noticed, type was taken. Your suggestions seem >> fine. We could also just do away with the macro as discussed earlier (we >> already do in the commit_node case, anyway...). > > Thinking on this more, writing out the definitions is the only sane > thing to do here, now that alloc_commit_node does not use the macro. > Otherwise you are inviting people to modify the macro, but fail to > notice that the commit allocator also needs updating. Hmm, well I could argue that using the macro for all allocators, apart from alloc_commit_node(), clearly shows which allocator is the odd-man out (and conversely, that all others are the same)! :-P No, I don't think this is a telling advantage; I don't think it makes that much difference. (six of one, half-a-dozen of the other). BTW, I tested the previous series on Linux 32-bit, Cygwin 32-bit, MinGW 32-bit and Cygwin 64-bit. (I can't test on Linux 64-bit, since I can't get Linux installed on my new laptop :( ). Admittedly, the testing on MinGW and Cygwin was only fairly light (it takes *hours* to run the full testsuite, and I just don't have the time). I was slightly concerned, when reading through this new series, that the alloc_node() function may no longer be inlined in the new allocators. However, I have just tested on Linux (only using gcc this time), and it was just fine. I will test the new series on the above systems later (probably tomorrow) but don't expect to find any problems. > > Here's a re-roll. The interesting bit is the addition of the second > patch (but the rest needed to be rebased on top). Yep, this looks good. Thanks! > > [1/8]: alloc.c: remove the alloc_raw_commit_node() function > [2/8]: alloc: write out allocator definitions > [3/8]: move setting of object->type to alloc_* functions > [4/8]: parse_object_buffer: do not set object type > [5/8]: add object_as_type helper for casting objects > [6/8]: alloc: factor out commit index > [7/8]: object_as_type: set commit index > [8/8]: diff-tree: avoid lookup_unknown_object ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html