Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module

[jim.cromie@gmail.com]

On Thu, Dec 11, 2025 at 8:14 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
>
> On 12/10/25 1:33 AM, jim.cromie@gmail.com wrote:
> > !-------------------------------------------------------------------|
> >    This Message Is From an External Sender
> >    This message came from outside your organization.
> > |-------------------------------------------------------------------!
> >
> > On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
> >>
> >> Hi Jim,
> >>
> >> Very minor nit below about the kernel-doc ordering for args...
> >>
> >
> >>> +/*
> >>> + * Walk the @_box->@_vec member, over @_vec.start[0..len], and find
> >>> + * the contiguous subrange of elements matching on ->mod_name.  Copy
> >>> + * the subrange into @_dst.  This depends on vars defd by caller.
> >>> + *
> >>> + * @_i:   caller provided counter var, init'd by macro
> >>> + * @_sp:  cursor into @_vec.
> >>> + * @_box: contains member named @_vec
> >>> + * @_vec: member-name of a type with: .start .len fields.
> >>> + * @_dst: an array-ref: to remember the module's subrange
> >>> + */
> >>
> >> Not sure if the odering matters for the docs, but it makes it a bit
> >> harder read when these don't go in order.
> >>
> >> Thanks,
> >>
> >> -Jason
> >>
> >
> > I chose that doc ordering for clarity,  the easy ones 1st,
> > and @dst last since it gets the subrange info.
> > I think reordering might mean more words trying to connect
> > the pieces, and with less clarity.
> > It does work against the macro arg ordering,
> > which places @dst near the front,
> > I did that to follow  LHS = RHS(...)   convention.
> >
> > Im happy to swap it around if anyone thinks that convention
> > should supercede these reasons,
> > but Im in NZ on vacation right now,
> > and I forgot to pull the latest rev off my desktop before I left.
> > so I dont want to fiddle with the slightly older copy I have locally,
> > and then have to isolate and fix whatever is different.
> >
> > the same applies to the Documentation tweaks that Bagas noted.
>
> Couldn't you then re-order the function args to match the doc order instead?
>

As you might surmise, the code was written before the kdoc.
Since it is setting the @_dst, it feels like an assignment.
Therefore the LHS = RHS convention seemed pertinent,
and the macro args are ordered to conform to this.
For the (pseudo- since its not /** ) kdoc,
the linear explanation was simplest and clearest, ending with @_dst.

So I see these options (in my preferred order), please pick one.
1. leave as is
2. add an NB: that arg order differs from doc-order
3. change macro arg order
4. change kdoc arg order

If 2-4 can wait, I can do that trivially once Im home (in Jan)
Doing it now, from here, will require fiddling with git am on the mbox.gz
with which Ive had mixed results/troubles in the past.

thanks,
Jim

> Thanks,
>
> -Jason
>
>
> >
> >
> >
> >
> >
> >>> +#define dd_mark_vector_subrange(_i, _dst, _sp, _box, _vec) ({                \
> >>> +     typeof(_dst) __dst = (_dst);                                    \
> >>> +     int __nc = 0;                                                   \
> >>> +     for_subvec(_i, _sp, _box, _vec) {                               \
> >>> +             if (!strcmp((_sp)->mod_name, (_dst)->mod_name)) {       \
> >>> +                     if (!__nc++)                                    \
> >>> +                             (__dst)->info._vec.start = (_sp);       \
> >>> +             } else {                                                \
> >>> +                     if (__nc)                                       \
> >>> +                             break; /* end of consecutive matches */ \
> >>> +             }                                                       \
> >>> +     }                                                               \
> >>> +     (__dst)->info._vec.len = __nc;                                  \
> >>> +})
> >>> +
> >>>    /*
> >>>     * Allocate a new ddebug_table for the given module
> >>>     * and add it to the global list.
> >>> @@ -1278,6 +1283,8 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
> >>>    static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
> >>>    {
> >>>        struct ddebug_table *dt;
> >>> +     struct _ddebug_class_map *cm;
> >>> +     int i;
> >>>
> >>>        if (!di->descs.len)
> >>>                return 0;
> >>> @@ -1300,6 +1307,8 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
> >>>
> >>>        INIT_LIST_HEAD(&dt->link);
> >>>
> >>> +     dd_mark_vector_subrange(i, dt, cm, di, maps);
> >>> +
> >>>        if (di->maps.len)
> >>>                ddebug_attach_module_classes(dt, di);
> >>>
> >>
>