Quoting Jani Nikula (2025-01-22 11:02:31-03:00) >On Wed, 22 Jan 2025, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote: >> Quoting Simona Vetter (2025-01-22 08:11:53-03:00) >>>On Tue, Jan 21, 2025 at 06:09:25PM -0300, Gustavo Sousa wrote: >>>> The header drm_print.h uses members of struct drm_device pointers, as >>>> such, it should include drm_device.h to let the compiler know the full >>>> type definition. >>>> >>>> Without such include, users of drm_print.h that don't explicitly need >>>> drm_device.h would bump into build errors and be forced to include the >>>> latter. >>>> >>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx> >>>> --- >>>> include/drm/drm_print.h | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >>>> index f77fe1531cf8..9732f514566d 100644 >>>> --- a/include/drm/drm_print.h >>>> +++ b/include/drm/drm_print.h >>>> @@ -32,6 +32,7 @@ >>>> #include <linux/dynamic_debug.h> >>>> >>>> #include <drm/drm.h> >>>> +#include <drm/drm_device.h> >>> >>>We much prefer just a struct device forward decl to avoid monster headers. >>>Is that not doable here? >> >> I don't think so. This header explicitly uses members of struct >> drm_device, so the compiler needs to know the full type definition. As >> an example see the definition of drm_WARN(), it uses (drm)->dev. > >I grudgingly agree. I don't think there are actual cases where this >happens, but I can imagine you could create one. It happened to me, and that motivated me to send this patch. I had a local patch where I just needed the drm_print.h header, but I ended up having to include drm_device.h in my .c file. > >>> Worst case I'd convert the drm_info_printer() static inline to a >>> macro, not sure about the exact rules here if you never deref a >>> pointer. > >The forward declaration is enough for passing pointers around without >dereferencing. It's the dereferencing in the macros that could fail the >build if the .c using them doesn't include drm_device.h. > >To balance things out, I think we could probably drop drm/drm.h if we >inlined one use of DRM_NAME to just "drm". > > >BR, >Jani. > > >>>-Sima >>> >>>> >>>> struct debugfs_regset32; >>>> struct drm_device; >>>> -- >>>> 2.48.1 >>>> >>> >>>-- >>>Simona Vetter >>>Software Engineer, Intel Corporation >>>http://blog.ffwll.ch > >-- >Jani Nikula, Intel
