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

[Jason Baron]

On 12/10/25 1:33 AM, jim.cromie@xxxxxxxxx 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@xxxxxxxxxx> 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?

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);