Date: Thu, 3 Oct 2019 20:43:53 -0600 From: Warner Losh <imp@bsdimp.com> To: Mike Karels <mike@karels.net> Cc: "Conrad E. Meyer" <cem@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: Dropping the type argument from the mtod macro Message-ID: <CANCZdfpXd_yFWMHHZRKbnTWGmC2w9WtS4BGV=LDb3ovdH3KrpQ@mail.gmail.com> In-Reply-To: <201910040210.x942A1pu029152@mail.karels.net> References: <CAG6CVpV2j=P6cjDpg46xy2-oZwxjB6hHF61V8m6BNmn1AffXEA@mail.gmail.com> <201910040210.x942A1pu029152@mail.karels.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 3, 2019, 8:10 PM Mike Karels <mike@karels.net> wrote: > > Hi all, > > > (net@ is BCC'd to keep discussion on arch@.) > > > 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@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpXd_yFWMHHZRKbnTWGmC2w9WtS4BGV=LDb3ovdH3KrpQ>