| Message ID | 20190522221237.19400-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 1f32abb995a04b7c67af6d923fea82436e817de1 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, this looks nicer indeed! Thanks j On Thu, May 23, 2019 at 01:12:37AM +0300, Laurent Pinchart wrote: > Now that the Camera class inherits from std::enable_shared_from_this, we > don't need to use std::allocate_shared anymore and can simplify the > Camera::create() implementation. This fixes compilation with recent > versions of libc++ whose std::allocate_shared implementation isn't > compatible with classes that are not publicly constructible. > > The custom allocator is removed, but a custom deleter is needed as the > Camera destructor is private. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera.cpp | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 0f45ab7e4358..617ea99cdf71 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -358,24 +358,17 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe, > const std::string &name, > const std::set<Stream *> &streams) > { > - struct Allocator : std::allocator<Camera> { > - void construct(void *p, PipelineHandler *pipe, > - const std::string &name) > + struct Deleter : std::default_delete<Camera> { > + void operator()(Camera *camera) > { > - ::new(p) Camera(pipe, name); > - } > - void destroy(Camera *p) > - { > - p->~Camera(); > + delete camera; > } > }; > > - std::shared_ptr<Camera> camera = > - std::allocate_shared<Camera>(Allocator(), pipe, name); > - > + Camera *camera = new Camera(pipe, name); > camera->streams_ = streams; > > - return camera; > + return std::shared_ptr<Camera>(camera, Deleter()); > } > > /** > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On Thu, May 23, 2019 at 09:53:37AM +0200, Jacopo Mondi wrote: > Hi Laurent, > this looks nicer indeed! I thing I've lost one: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > > On Thu, May 23, 2019 at 01:12:37AM +0300, Laurent Pinchart wrote: > > Now that the Camera class inherits from std::enable_shared_from_this, we > > don't need to use std::allocate_shared anymore and can simplify the > > Camera::create() implementation. This fixes compilation with recent > > versions of libc++ whose std::allocate_shared implementation isn't > > compatible with classes that are not publicly constructible. > > > > The custom allocator is removed, but a custom deleter is needed as the > > Camera destructor is private. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/camera.cpp | 17 +++++------------ > > 1 file changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 0f45ab7e4358..617ea99cdf71 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -358,24 +358,17 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe, > > const std::string &name, > > const std::set<Stream *> &streams) > > { > > - struct Allocator : std::allocator<Camera> { > > - void construct(void *p, PipelineHandler *pipe, > > - const std::string &name) > > + struct Deleter : std::default_delete<Camera> { > > + void operator()(Camera *camera) > > { > > - ::new(p) Camera(pipe, name); > > - } > > - void destroy(Camera *p) > > - { > > - p->~Camera(); > > + delete camera; > > } > > }; > > > > - std::shared_ptr<Camera> camera = > > - std::allocate_shared<Camera>(Allocator(), pipe, name); > > - > > + Camera *camera = new Camera(pipe, name); > > camera->streams_ = streams; > > > > - return camera; > > + return std::shared_ptr<Camera>(camera, Deleter()); > > } > > > > /** > > -- > > Regards, > > > > Laurent Pinchart > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Thu, May 23, 2019 at 09:53:37AM +0200, Jacopo Mondi wrote: > Hi Laurent, > this looks nicer indeed! Not just that, but it now compiles :-) > On Thu, May 23, 2019 at 01:12:37AM +0300, Laurent Pinchart wrote: > > Now that the Camera class inherits from std::enable_shared_from_this, we > > don't need to use std::allocate_shared anymore and can simplify the > > Camera::create() implementation. This fixes compilation with recent > > versions of libc++ whose std::allocate_shared implementation isn't > > compatible with classes that are not publicly constructible. > > > > The custom allocator is removed, but a custom deleter is needed as the > > Camera destructor is private. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/camera.cpp | 17 +++++------------ > > 1 file changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 0f45ab7e4358..617ea99cdf71 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -358,24 +358,17 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe, > > const std::string &name, > > const std::set<Stream *> &streams) > > { > > - struct Allocator : std::allocator<Camera> { > > - void construct(void *p, PipelineHandler *pipe, > > - const std::string &name) > > + struct Deleter : std::default_delete<Camera> { > > + void operator()(Camera *camera) > > { > > - ::new(p) Camera(pipe, name); > > - } > > - void destroy(Camera *p) > > - { > > - p->~Camera(); > > + delete camera; > > } > > }; > > > > - std::shared_ptr<Camera> camera = > > - std::allocate_shared<Camera>(Allocator(), pipe, name); > > - > > + Camera *camera = new Camera(pipe, name); > > camera->streams_ = streams; > > > > - return camera; > > + return std::shared_ptr<Camera>(camera, Deleter()); > > } > > > > /**
Hi Laurent, On 23/05/2019 09:10, Laurent Pinchart wrote: > Hi Jacopo, > > On Thu, May 23, 2019 at 09:53:37AM +0200, Jacopo Mondi wrote: >> Hi Laurent, >> this looks nicer indeed! > > Not just that, but it now compiles :-) Well we can't argue with that then :-) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> On Thu, May 23, 2019 at 01:12:37AM +0300, Laurent Pinchart wrote: >>> Now that the Camera class inherits from std::enable_shared_from_this, we >>> don't need to use std::allocate_shared anymore and can simplify the >>> Camera::create() implementation. This fixes compilation with recent >>> versions of libc++ whose std::allocate_shared implementation isn't >>> compatible with classes that are not publicly constructible. >>> >>> The custom allocator is removed, but a custom deleter is needed as the >>> Camera destructor is private. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> --- >>> src/libcamera/camera.cpp | 17 +++++------------ >>> 1 file changed, 5 insertions(+), 12 deletions(-) >>> >>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp >>> index 0f45ab7e4358..617ea99cdf71 100644 >>> --- a/src/libcamera/camera.cpp >>> +++ b/src/libcamera/camera.cpp >>> @@ -358,24 +358,17 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe, >>> const std::string &name, >>> const std::set<Stream *> &streams) >>> { >>> - struct Allocator : std::allocator<Camera> { >>> - void construct(void *p, PipelineHandler *pipe, >>> - const std::string &name) >>> + struct Deleter : std::default_delete<Camera> { >>> + void operator()(Camera *camera) >>> { >>> - ::new(p) Camera(pipe, name); >>> - } >>> - void destroy(Camera *p) >>> - { >>> - p->~Camera(); >>> + delete camera; >>> } >>> }; >>> >>> - std::shared_ptr<Camera> camera = >>> - std::allocate_shared<Camera>(Allocator(), pipe, name); >>> - >>> + Camera *camera = new Camera(pipe, name); >>> camera->streams_ = streams; >>> >>> - return camera; >>> + return std::shared_ptr<Camera>(camera, Deleter()); >>> } >>> >>> /** >
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 0f45ab7e4358..617ea99cdf71 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -358,24 +358,17 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe, const std::string &name, const std::set<Stream *> &streams) { - struct Allocator : std::allocator<Camera> { - void construct(void *p, PipelineHandler *pipe, - const std::string &name) + struct Deleter : std::default_delete<Camera> { + void operator()(Camera *camera) { - ::new(p) Camera(pipe, name); - } - void destroy(Camera *p) - { - p->~Camera(); + delete camera; } }; - std::shared_ptr<Camera> camera = - std::allocate_shared<Camera>(Allocator(), pipe, name); - + Camera *camera = new Camera(pipe, name); camera->streams_ = streams; - return camera; + return std::shared_ptr<Camera>(camera, Deleter()); } /**
Now that the Camera class inherits from std::enable_shared_from_this, we don't need to use std::allocate_shared anymore and can simplify the Camera::create() implementation. This fixes compilation with recent versions of libc++ whose std::allocate_shared implementation isn't compatible with classes that are not publicly constructible. The custom allocator is removed, but a custom deleter is needed as the Camera destructor is private. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/camera.cpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)