Messages in this thread | | | Date | Fri, 9 Sep 2022 19:58:45 +0900 | Subject | Re: [PATCH v9 2/8] util_macros: Add exact_type macro to catch type mis-match while compiling | From | Gwan-gyeong Mun <> |
| |
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 <gwan-gyeong.mun@intel.com> >> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >> Cc: Matthew Auld <matthew.auld@intel.com> >> Cc: Nirmoy Das <nirmoy.das@intel.com> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Andi Shyti <andi.shyti@linux.intel.com> >> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> >> --- >> 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. :) >
| |