[DTrace-devel] [PATCH v2 07/11] htab reduction: kernpath

[Nick Alcock]

On 22 Oct 2021, Kris Van Hees told this:

> On Fri, Oct 22, 2021 at 03:45:49PM -0400, Eugene Loh wrote:
>> On 10/22/21 3:09 PM, Kris Van Hees wrote:
>> 
>> > On Fri, Oct 22, 2021 at 12:31:56PM -0400, Eugene Loh wrote:
>> >> On 10/22/21 1:39 AM, Kris Van Hees wrote:
>> >>
>> >>> On Fri, Oct 22, 2021 at 12:15:51AM -0400, Eugene Loh wrote:
>> >>>> There is, however, some other consistency that might be reasonable in
>> >>>> these various patches.  E.g., in dt_htab.h, del always comes before
>> >>>> next.  So, how about using the same order in other files as well -- that
>> >>>> is, wherever "dt_htab_ops_t *_htab_ops" are defined.
>> >>>>
>> >>>> But come to think of it, when you define delete functions that free
>> >>>> resources, you append customized suffixes:  del_buf, del_path, del_mod,
>> >>>> and del_prov.  Why?  How about making them all del_res (or something).
>> >>>> Then, the dt_htab_ops_t definitions can all use a common macro:
>> >>>> DEFINE_HTAB_STD_OPS_RES(ID) (or something like that).
>> >>> Why not simply del?  Whether a delete function has other resources to free
>> >>> or not shouldn't matter to the htab implementation.  You call the op to
>> >>> delete the element, and the implementation of the ops will do what is needed
>> >>> for that particular type.
>> >> Hmm, yeah, good point.  But the delete function has the
>> >> doubly-linked-list pointer manipulation to do in any case, and that is
>> >> set up by the DEFINE_HE_STD_LINK_FUNCS macro.  So, I think the idea is
>> >> to take advantage of that boilerplate setup and to add a differently
>> >> named function as well in the case that there are other resources to
>> >> free.  Otherwise, one is defining a del() function that cuts and pastes
>> >> lots of pointer management in.
>> > Well, you can still define the standard link functions.  And then define a
>> > cusstom del_*() function that calls the *_del() function for the linking
>> > work to be done, and does the resource freeing.  And register that del_*()
>> > custom one as the .del hook.
>> Yes.  I think that's exactly what Nick does.  My point was only that if 

Yep, that's what this is all doing: adding a link function that wraps
around the standard one and deletes the actual thing pointed to, making
these htabs own their elements.

(It's really neat that this htab works both for owns-their-elements and
does-not-own-their-elements use cases: I think it's the first I've ever
seen that can do this so neatly. Obviously this adds a bit of difficulty
to use, because it means the caller has to know which sort of htab this
is, but that's the price of flexibility, and it's better than having
entirely separate APIs for all of it.)

>> such custom functions had a uniform name (rather than del_mod, del_prov, 
>> del_path, etc.), then the registrations could all use a common macro 
>> (some adaptation of DEFINE_HTAB_STD_OPS).
>
> Yes, but I do think that might be going a bit too far in providing higher level
> macros etc.  I think I prefer to see it more explicit in code rather than it
> being provided by the htab implementation - I have deliberately kept the
> implementation as generic as possible so adding in support for what elements
> may need beyond the htab related things is not really within the scope.

Agreed. I *am* kind of being consistent here: they're all called
del_FOO, where FOO is the sort of thing being deleted (where _del just
"deletes" the hash buckets themselves by resetting their link pointers).

I'm happy to make them all consistently named *_del_el or whatever if
people think that would be clearer. (I vacillated back and forth on this
at least twice.)

-- 
NULL && (void)