[libcamera-devel] [PATCH] FrameBufferAllocator: fix non-copyability

[Kieran Bingham]

Hi Tomi, Laurent,

On 25/09/2020 15:07, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Sep 25, 2020 at 05:05:25PM +0300, Tomi Valkeinen wrote:
>> FrameBufferAllocator is supposed to delete copy constructor and
>> copy-assignment operator. It doesn't do that as it uses Camera as a
>> parameter instead of FrameBufferAllocator.
> 
> Oops... Good catch.
> 
> It's a shame we can't have unit tests whose compilation failure would
> indicate success, to catch issues like these.

Should we perhaps make use of a macro to ensure this doesn't happen?
something akin to the Chromium DISALLOW_COPY_AND_ASSIGN type macros?

(Though, deleting them, rather than making them private).


>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at iki.fi>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

But for this,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
>> ---
>>  include/libcamera/framebuffer_allocator.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
>> index 78f1353..a96aaea 100644
>> --- a/include/libcamera/framebuffer_allocator.h
>> +++ b/include/libcamera/framebuffer_allocator.h
>> @@ -21,8 +21,8 @@ class FrameBufferAllocator
>>  {
>>  public:
>>  	FrameBufferAllocator(std::shared_ptr<Camera> camera);
>> -	FrameBufferAllocator(const Camera &) = delete;
>> -	FrameBufferAllocator &operator=(const Camera &) = delete;
>> +	FrameBufferAllocator(const FrameBufferAllocator &) = delete;
>> +	FrameBufferAllocator &operator=(const FrameBufferAllocator &) = delete;




>>  
>>  	~FrameBufferAllocator();
>>  
> 

-- 
Regards
--
Kieran