Date: Thu, 03 Oct 2019 21:10:01 -0500 From: Mike Karels <mike@karels.net> To: cem@freebsd.org Cc: "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: Dropping the type argument from the mtod macro Message-ID: <201910040210.x942A1pu029152@mail.karels.net> In-Reply-To: Your message of Sat, 28 Sep 2019 08:11:24 -0700. <CAG6CVpV2j=P6cjDpg46xy2-oZwxjB6hHF61V8m6BNmn1AffXEA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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. Mike
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910040210.x942A1pu029152>