[libcamera-devel] [PATCH] test: v4l2_device: Provide compile time assertions

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Jan 21, 2019 at 10:28:26AM +0000, Kieran Bingham wrote:
> The V4L2Capability structure is inherited from struct v4l2_capability only
> to provide an interface. It must not extend the type or data.
> 
> Enforce this with a static assertion with sizeof() comparisons.
> 
> There is no need here for a specific test binary which will always return
> TEST_PASS when compiled, as this test failure will be caught at compile time.
> In light of this - the static compile time assertion is added to the
> V4L2DeviceTest base class.
> 
> Should there be a large number of static assertions required, they could be
> moved to their own unit for clarity.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  test/v4l2_device/v4l2_device_test.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> index 362553712caa..97876f8d65db 100644
> --- a/test/v4l2_device/v4l2_device_test.cpp
> +++ b/test/v4l2_device/v4l2_device_test.cpp
> @@ -5,6 +5,7 @@
>   * libcamera V4L2 API tests
>   */
>  
> +#include <assert.h>
>  #include <iostream>
>  #include <sys/stat.h>
>  
> @@ -41,3 +42,8 @@ void V4L2DeviceTest::cleanup()
>  {
>  	delete dev_;
>  };
> +
> +/* Static compile time assertion tests */
> +
> +static_assert(sizeof(struct v4l2_capability) == sizeof(struct V4L2Capability),
> +	      "V4L2Capability must match v4l2_capability size");

I'm still not sure if we really need this, as this is more a check of
our C++ understanding than of our implementation :-) If you insist, I
prefer this check to be in a test case than in libcamera where it
originally was, so I won't push back too hard. The error message may
need some rewording though, as it sounds like it's something we can fix.

By the way, do we really require the two to be equal ? V4L2Capability
could include more members (I don't see a real use case for that now,
but that's imaginable), in which case it would be laid ou with
v4l2_capability first followed by other members, and still work fine.

-- 
Regards,

Laurent Pinchart