RE: [PATCH v13 3/3] drivers/net: add diagnostics macros to make code portable

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> Sent: Thursday, 16 January 2025 02.55
> 
> 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).
> 
> 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) ==
> 0))
>  				continue;
>  		}
> -		fd[num_rx] = (struct qbman_fd *)qbman_result_DQ_fd(dq_storage);
> +		fd[num_rx] = 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 = 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=msvc-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.