[libcamera-devel] [PATCH 3/5] libcamera: media_object: Utilise DELETE_COPY_AND_ASSIGN

[Laurent Pinchart]

On Fri, Oct 23, 2020 at 07:29:14AM +0300, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Oct 22, 2020 at 02:56:03PM +0100, Kieran Bingham wrote:
> > Convert MediaLink, MediaPad, and MediaEntity to declare DELETE_COPY_AND_ASSIGN.
> > These classes already deleted their copy constructor but not the assignment operator.
> > 
> > Utilising the DELETE_COPY_AND_ASSIGN prevents all copying of these classes.
> 
> Line wrap ?
> 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  include/libcamera/internal/media_object.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
> > index be6fb8961349..f467883072d2 100644
> > --- a/include/libcamera/internal/media_object.h
> > +++ b/include/libcamera/internal/media_object.h
> > @@ -12,6 +12,8 @@
> >  
> >  #include <linux/media.h>
> >  
> > +#include <libcamera/class.h>
> > +
> >  namespace libcamera {
> >  
> >  class MediaDevice;
> > @@ -50,7 +52,7 @@ private:
> >  
> >  	MediaLink(const struct media_v2_link *link,
> >  		  MediaPad *source, MediaPad *sink);
> > -	MediaLink(const MediaLink &) = delete;
> > +	DELETE_COPY_AND_ASSIGN(MediaLink);
> 
> You have placed DELETE_COPY_AND_ASSIGN() right after private: in patch
> 2/5. We could try and use the same placement in all classes for
> consistency. I don't mind much.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Actually I think we should disable both copy and move operations,
there's no need to move these classes. This is known as the rules of
five: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all

> >  	~MediaLink() {}
> >  
> >  	MediaPad *source_;
> > @@ -72,7 +74,7 @@ private:
> >  	friend class MediaDevice;
> >  
> >  	MediaPad(const struct media_v2_pad *pad, MediaEntity *entity);
> > -	MediaPad(const MediaPad &) = delete;
> > +	DELETE_COPY_AND_ASSIGN(MediaPad);
> >  	~MediaPad();
> >  
> >  	unsigned int index_;
> > @@ -104,7 +106,7 @@ private:
> >  
> >  	MediaEntity(MediaDevice *dev, const struct media_v2_entity *entity,
> >  		    unsigned int major = 0, unsigned int minor = 0);
> > -	MediaEntity(const MediaEntity &) = delete;
> > +	DELETE_COPY_AND_ASSIGN(MediaEntity);
> >  	~MediaEntity();
> >  
> >  	void addPad(MediaPad *pad);

-- 
Regards,

Laurent Pinchart