From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C52764609E; Thu, 16 Jan 2025 10:08:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1CDA04025F; Thu, 16 Jan 2025 10:08:12 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id A854F40156 for ; Thu, 16 Jan 2025 10:08:10 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 683B720679; Thu, 16 Jan 2025 10:08:10 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v13 3/3] drivers/net: add diagnostics macros to make code portable X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Thu, 16 Jan 2025 10:08:07 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F9BD@smartserver.smartshare.dk> In-Reply-To: <1736992511-20462-4-git-send-email-andremue@linux.microsoft.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v13 3/3] drivers/net: add diagnostics macros to make code portable Thread-Index: AdtnulRNzt9Pt9d1SpWsZc8hwdOYnwAMijmA References: <1735263196-2809-1-git-send-email-andremue@linux.microsoft.com> <1736992511-20462-1-git-send-email-andremue@linux.microsoft.com> <1736992511-20462-4-git-send-email-andremue@linux.microsoft.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Andre Muezerie" Cc: , , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Andre Muezerie [mailto:andremue@linux.microsoft.com] > Sent: Thursday, 16 January 2025 02.55 >=20 > It was a common pattern to have "GCC diagnostic ignored" pragmas > sprinkled over the code and only activate these pragmas for certain > compilers (gcc and clang). Clang supports GCC's pragma for > compatibility with existing source code, so #pragma GCC diagnostic > and #pragma clang diagnostic are synonyms for Clang > (https://clang.llvm.org/docs/UsersManual.html). >=20 > Now that effort is being made to make the code compatible with MSVC > these expressions would become more complex. It makes sense to hide > this complexity behind macros. This makes maintenance easier as these > macros are defined in a single place. As a plus the code becomes > more readable as well. Here is some food for thought and discussion... > @@ -2083,7 +2075,7 @@ dpaa2_dev_loopback_rx(void *queue, > if (unlikely((status & QBMAN_DQ_STAT_VALIDFRAME) =3D=3D > 0)) > continue; > } > - fd[num_rx] =3D (struct qbman_fd *)qbman_result_DQ_fd(dq_storage); > + fd[num_rx] =3D = RTE_PTR_DROP_QUALIFIERS(qbman_result_DQ_fd(dq_storage)); I do not think this makes the code more readable; quite the opposite. Before this, I could see which type the variable was being cast to. How about a macro that resembles "traditional" type casting: /** * Workaround to discard qualifiers (such as const, volatile, restrict) = from a pointer, * without the compiler emitting a warning. * * @warning * Although this macro can be abused for casting a pointer to point to a = different type, * alignment may be incorrect when casting to point to a larger type. = E.g.: * struct s { * uint16_t a; * uint8_t b; * uint8_t c; * uint8_t d; * } v; * uint16_t * p =3D RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not 16 = bit aligned! */ #define RTE_CAST_PTR(type, ptr) \ ((type)(uintptr_t)(ptr)) Writing the above warning lead me down another path... Can we somehow use __typeof_unqual__? It is available in both GCC [1] and MSVC [2]. [1]: https://gcc.gnu.org/onlinedocs/gcc/Typeof.html [2]: = https://learn.microsoft.com/en-us/cpp/c-language/typeof-unqual-c?view=3Dm= svc-170 We are making a workaround, and should take care to not endorse = overusing it. Especially for other purposes than intended. Unfortunately, I think some of the type casts don't just remove = qualifiers, but does exactly what my warning above describes: Casts a = pointer to completely different type. If the new type is a larger type, the pointer's alignment becomes = invalid, and if the compiler considers alignment a "qualifier", = -Wcast-qual emits a warning about it. Backtracking a bit... If the macro is intended to remove qualifiers, and not to cast to a = different type, RTE_PTR_DROP_QUALIFIERS(ptr) might be better than = RTE_CAST_PTR(type, ptr). For brevity and to resemble the C23 keyword typeof_unqual, it could be = named RTE_PTR_UNQUAL instead of RTE_PTR_DROP_QUALIFIERS.