| Message ID | 20241216104704.957343-2-pobrn@protonmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Mon, Dec 16, 2024 at 10:47:12AM +0000, Barnabás Pőcze wrote: > If `FrameBufferAllocator::allocate()` causes the construction to be > aborted, the allocated `GstLibcameraAllocator` will not be > deallocated properly. Use `g_autoptr()` to address this. > > `g_steal_pointer()` could only be used in glib 2.68 or later because > earlier it evaluates to a pointer-to-void in C++, which would necessitate > a `static_cast`. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/gstreamer/gstlibcameraallocator.cpp | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp > index b0c84893a..57405e083 100644 > --- a/src/gstreamer/gstlibcameraallocator.cpp > +++ b/src/gstreamer/gstlibcameraallocator.cpp > @@ -6,6 +6,8 @@ > * GStreamer Custom Allocator > */ > > +#include <utility> > + This should go after gstlibcameraallocator.h. Quoting codingstyle.rst, For .cpp files, if the file implements an API declared in a header file, that header file shall be included first in order to ensure it is self-contained. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > #include "gstlibcameraallocator.h" > > #include <libcamera/camera.h> > @@ -199,15 +201,13 @@ GstLibcameraAllocator * > gst_libcamera_allocator_new(std::shared_ptr<Camera> camera, > CameraConfiguration *config_) > { > - auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, > - nullptr)); > + g_autoptr(GstLibcameraAllocator) self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, > + nullptr)); > gint ret; > > self->cm_ptr = new std::shared_ptr<CameraManager>(gst_libcamera_get_camera_manager(ret)); > - if (ret) { > - g_object_unref(self); > + if (ret) > return nullptr; > - } > > self->fb_allocator = new FrameBufferAllocator(camera); > for (StreamConfiguration &streamCfg : *config_) { > @@ -228,7 +228,7 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera, > g_hash_table_insert(self->pools, stream, pool); > } > > - return self; > + return std::exchange(self, nullptr); > } > > bool
diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index b0c84893a..57405e083 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -6,6 +6,8 @@ * GStreamer Custom Allocator */ +#include <utility> + #include "gstlibcameraallocator.h" #include <libcamera/camera.h> @@ -199,15 +201,13 @@ GstLibcameraAllocator * gst_libcamera_allocator_new(std::shared_ptr<Camera> camera, CameraConfiguration *config_) { - auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, - nullptr)); + g_autoptr(GstLibcameraAllocator) self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, + nullptr)); gint ret; self->cm_ptr = new std::shared_ptr<CameraManager>(gst_libcamera_get_camera_manager(ret)); - if (ret) { - g_object_unref(self); + if (ret) return nullptr; - } self->fb_allocator = new FrameBufferAllocator(camera); for (StreamConfiguration &streamCfg : *config_) { @@ -228,7 +228,7 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera, g_hash_table_insert(self->pools, stream, pool); } - return self; + return std::exchange(self, nullptr); } bool
If `FrameBufferAllocator::allocate()` causes the construction to be aborted, the allocated `GstLibcameraAllocator` will not be deallocated properly. Use `g_autoptr()` to address this. `g_steal_pointer()` could only be used in glib 2.68 or later because earlier it evaluates to a pointer-to-void in C++, which would necessitate a `static_cast`. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/gstreamer/gstlibcameraallocator.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)