It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
compiling. The existing typecheck() macro outputs build warnings, but the
newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
a build break when the types are different and can be used to detect
explicit build errors.
v6: Move macro addition location so that it can be used by other than drm
subsystem (Jani, Mauro, Andi)
Signed-off-by: Gwan-gyeong Mun <[email protected]>
Cc: Thomas Hellström <[email protected]>
Cc: Matthew Auld <[email protected]>
Cc: Nirmoy Das <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Andi Shyti <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
---
include/linux/util_macros.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
index 72299f261b25..b6624b275257 100644
--- a/include/linux/util_macros.h
+++ b/include/linux/util_macros.h
@@ -2,6 +2,9 @@
#ifndef _LINUX_HELPER_MACROS_H_
#define _LINUX_HELPER_MACROS_H_
+#include <linux/types.h>
+#include <linux/bug.h>
+
#define __find_closest(x, a, as, op) \
({ \
typeof(as) __fc_i, __fc_as = (as) - 1; \
@@ -38,4 +41,26 @@
*/
#define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
+/**
+ * exact_type - break compile if source type and destination value's type are
+ * not the same
+ * @T: Source type
+ * @n: Destination value
+ *
+ * It is a helper macro for a poor man's -Wconversion: only allow variables of
+ * an exact type. It determines whether the source type and destination value's
+ * type are the same while compiling, and it breaks compile if two types are
+ * not the same
+ */
+#define exact_type(T, n) \
+ BUILD_BUG_ON(!__builtin_constant_p(n) && !__builtin_types_compatible_p(T, typeof(n)))
+
+/**
+ * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
+ * @n: value to compare pgoff_t type
+ *
+ * It breaks compile if the argument value's type is not pgoff_t type.
+ */
+#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
+
#endif
--
2.37.1
Hi Bartosz Golaszewski,
would you mind taking a look at this patch?
Thanks,
G.G.
On 8/24/22 5:45 PM, Gwan-gyeong Mun wrote:
> It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
> compiling. The existing typecheck() macro outputs build warnings, but the
> newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
> a build break when the types are different and can be used to detect
> explicit build errors.
>
> v6: Move macro addition location so that it can be used by other than drm
> subsystem (Jani, Mauro, Andi)
>
> Signed-off-by: Gwan-gyeong Mun <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Matthew Auld <[email protected]>
> Cc: Nirmoy Das <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Andi Shyti <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> ---
> include/linux/util_macros.h | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
> index 72299f261b25..b6624b275257 100644
> --- a/include/linux/util_macros.h
> +++ b/include/linux/util_macros.h
> @@ -2,6 +2,9 @@
> #ifndef _LINUX_HELPER_MACROS_H_
> #define _LINUX_HELPER_MACROS_H_
>
> +#include <linux/types.h>
> +#include <linux/bug.h>
> +
> #define __find_closest(x, a, as, op) \
> ({ \
> typeof(as) __fc_i, __fc_as = (as) - 1; \
> @@ -38,4 +41,26 @@
> */
> #define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
>
> +/**
> + * exact_type - break compile if source type and destination value's type are
> + * not the same
> + * @T: Source type
> + * @n: Destination value
> + *
> + * It is a helper macro for a poor man's -Wconversion: only allow variables of
> + * an exact type. It determines whether the source type and destination value's
> + * type are the same while compiling, and it breaks compile if two types are
> + * not the same
> + */
> +#define exact_type(T, n) \
> + BUILD_BUG_ON(!__builtin_constant_p(n) && !__builtin_types_compatible_p(T, typeof(n)))
> +
> +/**
> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
> + * @n: value to compare pgoff_t type
> + *
> + * It breaks compile if the argument value's type is not pgoff_t type.
> + */
> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
> +
> #endif
On Wed, Aug 24, 2022 at 05:45:08PM +0900, Gwan-gyeong Mun wrote:
> It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
> compiling. The existing typecheck() macro outputs build warnings, but the
> newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
> a build break when the types are different and can be used to detect
> explicit build errors.
>
> v6: Move macro addition location so that it can be used by other than drm
> subsystem (Jani, Mauro, Andi)
>
> Signed-off-by: Gwan-gyeong Mun <[email protected]>
> Cc: Thomas Hellstr?m <[email protected]>
> Cc: Matthew Auld <[email protected]>
> Cc: Nirmoy Das <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Andi Shyti <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> ---
> include/linux/util_macros.h | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
> index 72299f261b25..b6624b275257 100644
> --- a/include/linux/util_macros.h
> +++ b/include/linux/util_macros.h
> @@ -2,6 +2,9 @@
> #ifndef _LINUX_HELPER_MACROS_H_
> #define _LINUX_HELPER_MACROS_H_
>
> +#include <linux/types.h>
> +#include <linux/bug.h>
> +
> #define __find_closest(x, a, as, op) \
> ({ \
> typeof(as) __fc_i, __fc_as = (as) - 1; \
> @@ -38,4 +41,26 @@
> */
> #define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
>
> +/**
> + * exact_type - break compile if source type and destination value's type are
> + * not the same
> + * @T: Source type
> + * @n: Destination value
> + *
> + * It is a helper macro for a poor man's -Wconversion: only allow variables of
> + * an exact type. It determines whether the source type and destination value's
> + * type are the same while compiling, and it breaks compile if two types are
> + * not the same
> + */
> +#define exact_type(T, n) \
> + BUILD_BUG_ON(!__builtin_constant_p(n) && !__builtin_types_compatible_p(T, typeof(n)))
Maybe use __same_type() here instead of open-coded
__builtin_types_compatible_p()? Also, IIUC, currently coding style
advise is to use _Static_assert when possible over BUILD_BUG_ON for
error message readability.
This macro has a trap-door for literals, yes?
i.e. exact_type(pgoff_t, 5) will pass?
I also note that this is very close to the really common (and open-coded)
test scattered around the kernel already (BUILD_BUG_ON(__same_type(a,
b))), so I think it's good to get a macro defined for it, though I'm not
sure about the trap door test. Regardless, I'd like to bikeshed the name
a bit; I think this should be named something a bit more clear about
what happens on failure. Perhaps: assert_type()? Or to capture the
trapdoor idea, assert_typable()?
#define assert_type(t1, t2) _Static_assert(__same_type(t1, t2))
#define assert_typable(t, n) _Static_assert(__builtin_constant_p(n) ||
__same_type(t, typeof(n))
> +
> +/**
> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
> + * @n: value to compare pgoff_t type
> + *
> + * It breaks compile if the argument value's type is not pgoff_t type.
> + */
> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
Why specialize this? Just use assert_typable(pgoff_t, n) in the other
patches? It's almost the same amount to write. :)
--
Kees Cook
On 8/26/22 2:19 AM, Kees Cook wrote:
> On Wed, Aug 24, 2022 at 05:45:08PM +0900, Gwan-gyeong Mun wrote:
>> It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
>> compiling. The existing typecheck() macro outputs build warnings, but the
>> newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
>> a build break when the types are different and can be used to detect
>> explicit build errors.
>>
>> v6: Move macro addition location so that it can be used by other than drm
>> subsystem (Jani, Mauro, Andi)
>>
>> Signed-off-by: Gwan-gyeong Mun <[email protected]>
>> Cc: Thomas Hellström <[email protected]>
>> Cc: Matthew Auld <[email protected]>
>> Cc: Nirmoy Das <[email protected]>
>> Cc: Jani Nikula <[email protected]>
>> Cc: Andi Shyti <[email protected]>
>> Cc: Mauro Carvalho Chehab <[email protected]>
>> ---
>> include/linux/util_macros.h | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
>> index 72299f261b25..b6624b275257 100644
>> --- a/include/linux/util_macros.h
>> +++ b/include/linux/util_macros.h
>> @@ -2,6 +2,9 @@
>> #ifndef _LINUX_HELPER_MACROS_H_
>> #define _LINUX_HELPER_MACROS_H_
>>
>> +#include <linux/types.h>
>> +#include <linux/bug.h>
>> +
>> #define __find_closest(x, a, as, op) \
>> ({ \
>> typeof(as) __fc_i, __fc_as = (as) - 1; \
>> @@ -38,4 +41,26 @@
>> */
>> #define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
>>
>> +/**
>> + * exact_type - break compile if source type and destination value's type are
>> + * not the same
>> + * @T: Source type
>> + * @n: Destination value
>> + *
>> + * It is a helper macro for a poor man's -Wconversion: only allow variables of
>> + * an exact type. It determines whether the source type and destination value's
>> + * type are the same while compiling, and it breaks compile if two types are
>> + * not the same
>> + */
>> +#define exact_type(T, n) \
>> + BUILD_BUG_ON(!__builtin_constant_p(n) && !__builtin_types_compatible_p(T, typeof(n)))
>
> Maybe use __same_type() here instead of open-coded
> __builtin_types_compatible_p()? Also, IIUC, currently coding style
> advise is to use _Static_assert when possible over BUILD_BUG_ON for
> error message readability.
>
> This macro has a trap-door for literals, yes?
> i.e. exact_type(pgoff_t, 5) will pass?
>
yes, I will update in detail comments about trap-door that may occur
when using constant value.
> I also note that this is very close to the really common (and open-coded)
> test scattered around the kernel already (BUILD_BUG_ON(__same_type(a,
> b))), so I think it's good to get a macro defined for it, though I'm not
> sure about the trap door test. Regardless, I'd like to bikeshed the name
> a bit; I think this should be named something a bit more clear about
> what happens on failure. Perhaps: assert_type()? Or to capture the
> trapdoor idea, assert_typable()?
>
> #define assert_type(t1, t2) _Static_assert(__same_type(t1, t2))
> #define assert_typable(t, n) _Static_assert(__builtin_constant_p(n) ||
> __same_type(t, typeof(n))
The form of the assert_type() / assert_typable() macros you suggested
looks better to me, so I will add these macros to the header where
__same_type() is defined and will send a new version of the patch.
many thanks
>
>> +
>> +/**
>> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
>> + * @n: value to compare pgoff_t type
>> + *
>> + * It breaks compile if the argument value's type is not pgoff_t type.
>> + */
>> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
>
> Why specialize this? Just use assert_typable(pgoff_t, n) in the other
> patches? It's almost the same amount to write. :)
>