From owner-freebsd-arch@freebsd.org Fri Oct 4 02:10:08 2019 Return-Path: Delivered-To: freebsd-arch@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AC1BC146CB3 for ; Fri, 4 Oct 2019 02:10:08 +0000 (UTC) (envelope-from mike@karels.net) Received: from mail.karels.net (mail.karels.net [216.160.39.52]) by mx1.freebsd.org (Postfix) with ESMTP id 46ktcb5k6Cz3JSD; Fri, 4 Oct 2019 02:10:07 +0000 (UTC) (envelope-from mike@karels.net) Received: from mail.karels.net (localhost [127.0.0.1]) by mail.karels.net (8.15.2/8.15.2) with ESMTP id x942A1pu029152; Thu, 3 Oct 2019 21:10:01 -0500 (CDT) (envelope-from mike@karels.net) Message-Id: <201910040210.x942A1pu029152@mail.karels.net> To: cem@freebsd.org cc: "freebsd-arch@freebsd.org" From: Mike Karels Reply-to: mike@karels.net Subject: Re: Dropping the type argument from the mtod macro In-reply-to: Your message of Sat, 28 Sep 2019 08:11:24 -0700. MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <29150.1570155001.1@mail.karels.net> Date: Thu, 03 Oct 2019 21:10:01 -0500 X-Rspamd-Queue-Id: 46ktcb5k6Cz3JSD X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of mike@karels.net designates 216.160.39.52 as permitted sender) smtp.mailfrom=mike@karels.net X-Spamd-Result: default: False [-3.51 / 15.00]; ARC_NA(0.00)[]; TO_DN_EQ_ADDR_SOME(0.00)[]; HAS_REPLYTO(0.00)[mike@karels.net]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:216.160.39.52]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[karels.net]; REPLYTO_ADDR_EQ_FROM(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; IP_SCORE(-1.31)[ip: (-4.74), ipnet: 216.160.0.0/15(-1.69), asn: 209(-0.08), country: US(-0.05)]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:209, ipnet:216.160.0.0/15, country:US]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Oct 2019 02:10:08 -0000 > 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