On Wed, Mar 24, 2021 at 12:57:15PM -0700, Junio C Hamano wrote: > And while "type" could also be made a part of this structure and > have the API always iterate over all entries regardless of "type", > i.e. the callback function could be the one responsible for finding > the entries in the table for one particular type, it is understandable > that potential callers can be helped by having the pre-filtering > based on the "type" thing on the API side. Yeah. We _could_ also have the for-each function filter by name (if present). And then quit when it finds a matching name (because it knows the names are supposed to be unique). That is the opposite of: > Or a single for_each_driver() that gives <name, length, type> tuple > to the callback function, iterating over all drivers regardless of > the type. I'd be fine with that, too. I don't have a huge preference either way, but it does feel like it would fix the weird asymmetry (though as I said, I don't think the asymmetry is _wrong_, and it may not be worth over-engineering this tiny corner of the interface). Thanks for spelling out both of these directions clearly. My response to Ævar was a little muddled as I was having trouble figuring out just what it was that left me so puzzled by the patch. :) By the way, one thing I wondered about all of this: could we have an entry in both the custom and builtin lists? I think the answer is "no", because any custom config for a builtin type would get placed inside the existing struct in the builtin_drivers array (which is sadly a reason we must leak any old string values when we parse diff.*.funcname, etc; we don't know if they are string literals or heap values from previously parsed config). I also notice that the "builtin" for-each does not count the boolean "diff" or "!diff" attribute structs. That is perhaps reasonable, as they do not have interesting names nor values in the first place. I do still wonder whether "builtin vs custom" is even a useful distinction (i.e., those builtins are less builtin_drivers[] than drivers[]). -Peff
