On Sat, Dec 13, 2025 at 5:06 AM Jason Baron <[email protected]> wrote:
>
>
>
> On 12/10/25 3:21 PM, [email protected] wrote:
> > !-------------------------------------------------------------------|
> > This Message Is From an External Sender
> > This message came from outside your organization.
> > |-------------------------------------------------------------------!
> >
> > On Thu, Dec 11, 2025 at 8:14 AM Jason Baron <[email protected]> wrote:
> >>
> >>
> >>
> >> On 12/10/25 1:33 AM, [email protected] 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 <[email protected]> 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.
> >
>
> Hi Jim,
>
> I am fine leaving this as is, but I do feel like we should perhaps do at
> least #2 at some point, to clarify things.
>
>
Im redoing the set anyway, so I'll do either 2 or 3.
thx
> Thanks,
>
> -Jason
>
>
>
>
>
>
>
>
> > 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);
> >>>>>
> >>>>
> >>
>
