Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Macros xe_any_to_drm, xe_any_to_dev, and xe_any_to_pdev fail to compile when passed const pointers due to a C11 constraint violation in the underlying xe_any_to_xe macro.
- [Low] The macros xe_any_if_type and xe_any_id silently discard side effects for unmatched types.
--
--- Patch [1]: [PATCH v4 01/32] drm/xe: Introduce xe_any helpers ---
commit 933eba9892b749844655f0d89de25fa2cfc22f5b
Author: Michal Wajdeczko <[email protected]>
Subject: drm/xe: Introduce xe_any helpers
This commit introduces a set of macros using _Generic to convert between
xe_device, xe_tile, xe_gt, drm_device, pci_dev, and device pointers. It
provides type-checking helpers and object identifier retrievals across the
xe driver.
> diff --git a/drivers/gpu/drm/xe/xe_any.h b/drivers/gpu/drm/xe/xe_any.h
> new file mode 100644
> index 0000000000000..5d97afa76915a
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_any.h
[ ... ]
> +/**
> + * xe_any_if_type() - Get the pointer only if it is @type pointer.
> + * @any: any pointer
> + * @type: data type to look for
> + *
> + * Return: the @type pointer or NULL.
> + */
> +#define xe_any_if_type(any, type) \
> + _Generic((any), \
> + __xe_any_to_self_assoc(type, (any)), \
> + default : NULL)
[Severity: Low]
Could this macro discard side effects for unmatched types?
If a caller passes an expression with side effects, like a function call or
pointer increment, to xe_any_if_type() or any macro that uses it, the side
effects will be evaluated if the type matches.
However, if the type falls to the default branch, the side effects will be
completely discarded because (any) is not evaluated in the default : NULL
association.
[ ... ]
> +/**
> + * xe_any_to_xe() - Obtain the &xe_device pointer.
> + * @any: the &pci_dev or the &xe_device or &xe_tile or &xe_gt pointer
> + *
> + * Return: the @xe_device pointer or backpointer.
> + */
> +#define xe_any_to_xe(any) \
> + _Generic((any), \
> + __xe_any_to_self_assoc(struct xe_device, (any)), \
> + __xe_any_to_other_assoc(/* */, xe_tile, xe_device, (any)), \
> + __xe_any_to_other_assoc(const, xe_tile, xe_device, (any)), \
> + __xe_any_to_other_assoc(/* */, xe_gt, xe_device, (any)), \
> + __xe_any_to_other_assoc(const, xe_gt, xe_device, (any)), \
> + __xe_any_to_other_assoc(, drm_device, xe_device, (any)), \
> + __xe_any_to_other_assoc(, pci_dev, xe_device, (any)), \
> + __xe_any_to_other_assoc(, device, xe_device, (any)))
> +
> +/**
> + * xe_any_to_drm() - Obtain the &drm_device pointer.
> + * @any: the &pci_dev or the &xe_device or &xe_tile or &xe_gt pointer
> + *
> + * Return: the @drm_device pointer or backpointer.
> + */
> +#define xe_any_to_drm(any) \
> + _Generic((any), \
> + __xe_any_to_self_assoc(struct drm_device, (any)), \
> + default : &xe_any_to_xe(any)->drm)
[Severity: Low]
Will this cause a C11 constraint violation during compilation if the parameter
is a const struct drm_device pointer?
When xe_any_to_drm is called with a const struct drm_device pointer, it falls
to the default branch which contains xe_any_to_xe(any).
The xe_any_to_xe macro lacks const associations for drm_device, pci_dev, and
device, and it does not have a default branch. This will cause compilation to
fail even though it is inside an unselected branch of the outer _Generic block.
[ ... ]
> +/**
> + * xe_any_id() - Get the identifier of the underlying object.
> + * @any: the &pci_dev or the &xe_device or &xe_tile or &xe_gt pointer
> + *
> + * Return: the identifier of the object, or 0 if not applicable/available.
> + */
> +#define xe_any_id(any) \
> + _Generic((any), \
> + __xe_any_to_other_assoc(/* */, xe_tile, id, (any)), \
> + __xe_any_to_other_assoc(const, xe_tile, id, (any)), \
> + __xe_any_to_other_assoc(/* */, xe_gt, id, (any)), \
> + __xe_any_to_other_assoc(const, xe_gt, id, (any)), \
> + default : 0)
[Severity: Low]
Does this macro also discard side effects for unmatched types, similar to
xe_any_if_type?
If (any) is an expression with side effects, they are evaluated if the type
matches, but are lost in the default : 0 branch because (any) is omitted.
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.