[libcamera-devel] [PATCH 16/16] test: Ensure LIBCAMERA_BASE_PRIVATE isn't public

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Jun 25, 2021 at 10:44:16AM +0100, Kieran Bingham wrote:
> On 25/06/2021 10:05, paul.elder at ideasonboard.com wrote:
> > On Fri, Jun 25, 2021 at 02:35:39AM +0100, Kieran Bingham wrote:
> >> If LIBCAMERA_BASE_PRIVATE is ever exposed on the libcamera public dependencies,
> >> then the private.h header protection will be circumvented.
> >>
> >> Provide a test which will fail (at compile time) if the LIBCAMERA_BASE_PRIVATE
> >> define ever leaks to the public dependencies.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >> ---
> >>  test/meson.build    |  1 +
> >>  test/public-api.cpp | 25 +++++++++++++++++++++++++
> >>  2 files changed, 26 insertions(+)
> >>  create mode 100644 test/public-api.cpp
> >>
> >> diff --git a/test/meson.build b/test/meson.build
> >> index 73eb44d03ad0..2c3e76546fbc 100644
> >> --- a/test/meson.build
> >> +++ b/test/meson.build
> >> @@ -25,6 +25,7 @@ subdir('v4l2_videodevice')
> >>  
> >>  public_tests = [
> >>      ['geometry',                        'geometry.cpp'],
> >> +    ['public-api',                      'public-api.cpp'],
> >>      ['signal',                          'signal.cpp'],
> >>      ['span',                            'span.cpp'],
> >>  ]
> >> diff --git a/test/public-api.cpp b/test/public-api.cpp
> >> new file mode 100644
> >> index 000000000000..5afce97c6887
> >> --- /dev/null
> >> +++ b/test/public-api.cpp
> >> @@ -0,0 +1,25 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2021, Google Inc.
> >> + *
> >> + * public-api.cpp - Public API validation
> >> + */
> >> +
> >> +#include <libcamera/libcamera.h>
> >> +
> >> +#include "test.h"
> >> +
> >> +class PublicAPITest : public Test
> >> +{
> >> +	int run()
> >> +	{
> >> +#ifdef LIBCAMERA_BASE_PRIVATE
> >> +#error "Public Interfaces should not be exposed to LIBCAMERA_BASE_PRIVATE"

s/Interfaces/interfaces/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > s/be exposed to/expose/ ?
> 
> Hrm, that's not actually my intention. It would check both cases though.
> It's subtle, ...
> 
> My intention is to prevent someone adding or changing the meson
> dependencies such that somehow libcamera_base_private ends up being a
> part of libcamera_public dependency which would then magically make all
> public interfaces compile with LIBCAMERA_BASE_PRIVATE defined, and that
> would then break all the protection.
> 
> But your change, would also be correct and something to protect against
> (and which this does protect against) but it's not something I would
> expect to happen, as that would imply someone has added a #define
> LIBCAMERA_BASE_PRIVATE within one of the API headers (which would then
> have the same break.
> 
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > 
> >> +		return TestFail;
> >> +#else
> >> +		return TestPass;
> >> +#endif
> >> +	}
> >> +};
> >> +
> >> +TEST_REGISTER(PublicAPITest)

-- 
Regards,

Laurent Pinchart