Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] drm/atomic: incorrect field name stringified in STATE_CHECK static_asserts - [Low] drm/atomic: format string mismatch in STATE_CHECK_U32_16_16 macro --
commit 82f5bb4389d99e46549406b2a6e1384ec96ce6cc Author: Maxime Ripard <[email protected]> drm/atomic-state-helper: Provide comparison macros In order to make the implementation of atomic_sro_compare_state easier and reduce the boilerplate, introduce a set of STATE_CHECK_* macros to compare state field values depending on their type. > diff --git a/include/drm/drm_atomic_sro_helper.h > b/include/drm/drm_atomic_sro_helper.h > index a42ed8136a499..71340978ce7c4 100644 > --- a/include/drm/drm_atomic_sro_helper.h > +++ b/include/drm/drm_atomic_sro_helper.h [ ... ] > +#define STATE_CHECK_BOOL(r, p, n, sa, sb, f) \ > + do { \ > + static_assert(__same_type(sa->f, sb->f), \ > + __stringify(f) " field types don't match"); \ > + static_assert(__same_type(sa->f, bool), \ > + __stringify(n) " is not a bool"); \ ^^^^^^^^^^^^^^ [Severity: Low] Is this static_assert using the wrong parameter for the stringified message? Since 'n' is the object's human-readable name and 'f' is the field name, if a type mismatch occurs, the compiler will emit a message like "plane is not a bool" instead of the intended field name. Should this use __stringify(f) instead of __stringify(n)? This pattern appears to be repeated across all the STATE_CHECK_* macros. [ ... ] > +#define STATE_CHECK_U32_16_16(r, p, n, sa, sb, f) \ > + do { \ > + static_assert(__same_type(sa->f, sb->f), \ > + __stringify(f) " field types don't match"); \ > + static_assert(__same_type(sa->f, (u32)0), \ > + __stringify(n) " is not a u32"); \ > + if (sa->f != sb->f) { \ > + drm_atomic_helper_print_state_mismatch(p, \ > + n, \ > + __stringify(f), \ > + "expected > %d.%06u, got %d.%06u", \ ^^ ^^ [Severity: Low] Does this format string mismatch the argument types? The expression 'sa->f >> 16' produces an unsigned int (since sa->f is u32), but the format string uses '%d' instead of '%u'. This can trigger a -Wformat compiler warning. > + sa->f >> 16, > ((sa->f & 0xffff) * 15625) >> 10, \ > + sb->f >> 16, > ((sb->f & 0xffff) * 15625) >> 10); \ > + r = false; \ > + } \ > + } while (0) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
