I updated the patch earlier today incorporating the suggestions. I also had to bring back "#include <linux/types.h>" to drm.h because there's some sanity check that fails, as if it doesn't scan past the first level of #includes..
- James
On Mon, Dec 7, 2020 at 3:14 AM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote:
On Mon, 07 Dec 2020 10:53:49 +0000
Simon Ser <contact@xxxxxxxxxxx> wrote:
> On Monday, December 7th, 2020 at 11:49 AM, James Park <james.park@xxxxxxxxxxxxxxxx> wrote:
>
> > That would work, but that's kind of an annoying requirement. I would
> > prefer the header to be self-sufficient.
>
> I don't want to make it more confusing than before, but here Pekka (I
> think) suggests to replace this:
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
> #ifndef DRM_FOURCC_H
> #define DRM_FOURCC_H
>
> +#ifdef DRM_FOURCC_STANDALONE
> +#include "drm_basic_types.h"
> +#else
> #include "drm.h"
> +#endif
>
> #if defined(__cplusplus)
> extern "C" {
>
> With this:
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
> #ifndef DRM_FOURCC_H
> #define DRM_FOURCC_H
>
> +#include "drm_basic_types.h"
> +
> +#ifndef DRM_FOURCC_STANDALONE
> #include "drm.h"
> +#endif
>
> #if defined(__cplusplus)
> extern "C" {
>
> That wouldn't change whether the header is self-sufficient or not,
> would it?
Exactly this.
This communicates properly that DRM_FOURCC_STANDALONE only affects
whether drm.h gets pulled in or not, and there are no other effects.
This also makes testing better: when you unconditionally include
drm_basic_types.h, you are more likely to catch breakage there.
For functionality, it makes no difference. Whether userspace does
#include "drm.h"
#define DRM_FOURCC_STANDALONE
#include "drm_fourcc.h"
or
#define DRM_FOURCC_STANDALONE
#include "drm_fourcc.h"
#include "drm.h"
the result must always be good.
Thanks,
pq
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
