Dropping the type argument from the mtod macro

[Warner Losh]

On Thu, Oct 3, 2019, 8:10 PM Mike Karels <mike at karels.net> wrote:

> > Hi all,
>
> > (net@ is BCC'd to keep discussion on arch at .)
>
> > In the past we've moved away from macros with explicit types as
> > parameters, in favor of casting void* results as needed (e.g.,
> > MALLOC()).
>
> > I'd like to do the same with mtod.  The refactoring is easy enough to
> > do safely with a Coccinelle semantic patch.  The semantic patch
> > converts instances of "mtod(m, t)" to "(t)mtod(m)".
>
> > markj@ and rrs@ both point out that the network stack uses the type
> > argument as a visual tool in large functions where the declaration of
> > the lvariable isn't necessarily nearby its use.  I think the
> > "(t)mtod(m)" preserves that utility as a visual tool; I may be
> > mistaken.
>
> > Currently, the semantic patch drops the explicit "(t)" in two limited
> > cases: "t" is "void *", or the "mtod()" invocation is used during
> > initial variable declaration and assignment.  The reasoning is that in
> > both cases, the "t" isn't giving you any additional information.
> > Neither of these exceptions is core to the idea, and either could be
> > removed.
>
> > What do you think?  A rough draft of the proposal is sketched out
> > here: https://reviews.freebsd.org/D21669
>
> > Best,
> > Conrad
>
> > P.S., On an earlier version of this revision, markj@ and emaste@ both
> > expressed concern for downstream consumers.  There are two aspects of
> > the current differential that should smooth that experience.  First, a
> > backwards-compatible two-argument mtod(m, t) is retained using an ugly
> > macro hack.  (Existing code compiles without modification.)  Second,
> > the coccinelle semantic patch will be checked in to the tree somewhere
> > for use by downstream consumers that want to convert their code.
>
> I was surprised not to see responses on the list, but I see that there
> was already a fair amount of discussion on the phabricator review up
> to the time of this message.  I'll reply on the list in hopes of getting
> more feedback.
>
> I like the proposed usage better than the original mtod(); I never liked
> the use of a type as a parameter.  However, I think it is questionable
> whether the cleanup is worth the cost, both for upstream and downstream.
> I think the biggest issue is that it makes FreeBSD code harder to use
> in other contexts; for example, Michael Tuexen pointed out the issue
> of sharing SCTP sources with other BSD-like platforms.  We'll end up
> with two or more different styles.  He asks, given the price to be paid,
> what do we gain?  On the other hand, mtod() in its old form is ubiquitous
> and easy to understand.
>

Clearly we need m2d() instead. A more modern name for a more modern API.

In all seriousness, though, I'm more drawn to the too little gain, too much
pain arguments more than the purity arguments.

Warner

                Mike
> _______________________________________________
> freebsd-arch at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"
>