Fix some issues highlighted by static checking

26670 diff mbox

Dear All,
First submission -- apologies if not following established procedures.
Compiles, but untested.

There may be better solutions to the enum versus flag changes.

Best regards,
Neil Matthews

From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001
From: Neil Matthews <drneildmatthews@googlemail.com>
Date: Fri, 1 May 2026 16:46:30 +0100
Subject: [PATCH] Fix some issues highlighted by static checking.

Checkers were invoked as:

cppcheck --enable=all --force --inconclusive

and as

bear ninja -C build
find . -name "*.cpp" -exec clang-tidy {} ';'

Issues raised include:
- using enums as flags that are or'ed together creating values not defined in the enum
- checking result of malloc
- checking (null) pointers before dereferencing
- avoiding strcat()

Other flagged issues remain to be reviewed and fixed if required.

Signed-off-by: Neil Matthews <drneildmatthews@googlemail.com>
---
 include/libcamera/base/memfd.h                 |  1 +
 include/libcamera/controls.h                   |  1 +
 include/libcamera/internal/dma_buf_allocator.h |  2 ++
 src/android/metadata/camera_metadata.c         |  4 ++++
 src/libcamera/base/object.cpp                  |  3 +++
 src/libcamera/ipa_data_serializer.cpp          | 16 ++++++++++++----
 src/libcamera/pipeline/ipu3/ipu3.cpp           |  4 +---
 src/libcamera/pipeline/simple/simple.cpp       |  3 +++
 src/libcamera/software_isp/debayer_cpu.cpp     |  2 --
 src/libcamera/v4l2_pixelformat.cpp             |  7 ++++---
 10 files changed, 31 insertions(+), 12 deletions(-)

Hi Neil,

Quoting Neil Matthews (2026-05-07 12:23:16)
> Dear All,
> First submission -- apologies if not following established procedures.
> Compiles, but untested.
> 
> There may be better solutions to the enum versus flag changes.
> 
> Best regards,
> Neil Matthews
> 
> From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001
> From: Neil Matthews <drneildmatthews@googlemail.com>
> Date: Fri, 1 May 2026 16:46:30 +0100
> Subject: [PATCH] Fix some issues highlighted by static checking.
> 
> Checkers were invoked as:
> 
> cppcheck --enable=all --force --inconclusive
> 
> and as

Thank you for the cleanups.

Each one is likely suitable to be a single distinct patch with a clear
commit message to explain the issue and the update in the patch.

Please see: 

And I always recommend https://cbea.ms/git-commit/ as a great read for
how we like to structure and document commits 'atomically' such that
each commit performs a single purpose.

> 
> bear ninja -C build
> find . -name "*.cpp" -exec clang-tidy {} ';'
> 
> Issues raised include:
> - using enums as flags that are or'ed together creating values not defined in the enum

> - checking result of malloc
> - checking (null) pointers before dereferencing
> - avoiding strcat()

> 
> Other flagged issues remain to be reviewed and fixed if required.
> 
> Signed-off-by: Neil Matthews <drneildmatthews@googlemail.com>
> ---
>  include/libcamera/base/memfd.h                 |  1 +
>  include/libcamera/controls.h                   |  1 +
>  include/libcamera/internal/dma_buf_allocator.h |  2 ++
>  src/android/metadata/camera_metadata.c         |  4 ++++
>  src/libcamera/base/object.cpp                  |  3 +++
>  src/libcamera/ipa_data_serializer.cpp          | 16 ++++++++++++----
>  src/libcamera/pipeline/ipu3/ipu3.cpp           |  4 +---
>  src/libcamera/pipeline/simple/simple.cpp       |  3 +++
>  src/libcamera/software_isp/debayer_cpu.cpp     |  2 --
>  src/libcamera/v4l2_pixelformat.cpp             |  7 ++++---
>  10 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h
> index 5ffa0f22..6805d374 100644
> --- a/include/libcamera/base/memfd.h
> +++ b/include/libcamera/base/memfd.h
> @@ -21,6 +21,7 @@ public:
>                 None = 0,
>                 Shrink = (1 << 0),
>                 Grow = (1 << 1),
> +                ShrinkAndGrow = Shrink | Grow,
>         };

I'm not sure if these sort of detract from the purpose of using bit
fields here.

With this we'd have to make every combintion of bits in the enum?

>  
>         using Seals = Flags<Seal>;
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 44ff4964..a22fe28f 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -253,6 +253,7 @@ public:
>         enum class Direction {
>                 In = (1 << 0),
>                 Out = (1 << 1),
> +                InAndOut = In | Out,
>         };
>  
>         using DirectionFlags = Flags<Direction>;
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index 13600915..40dc36da 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -26,7 +26,9 @@ public:
>         enum class DmaBufAllocatorFlag {
>                 CmaHeap = 1 << 0,
>                 SystemHeap = 1 << 1,
> +               CmaHeapAndSystemHeap = CmaHeap | SystemHeap,
>                 UDmaBuf = 1 << 2,
> +               CmaHeapAndSystemHeapAndUDmaBuf = CmaHeap | SystemHeap | UDmaBuf,
>         };
>  
>         using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>;
> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> index b86586a7..89cc792f 100644
> --- a/src/android/metadata/camera_metadata.c
> +++ b/src/android/metadata/camera_metadata.c
> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(
>      }
>  
>      void *buffer = malloc(src_size);
> +    if(!buffer) {
> +        return NULL;
> +    }
> +

This would just be:

	if (!buffer)
		return NULL;

in our coding style, but I think in general we make the assumption
malloc can't fail? (I think I recall that just like we assume 'new' in
C++ always succeeds).



>      memcpy(buffer, src, src_size);
>  
>      camera_metadata_t *metadata = (camera_metadata_t*) buffer;
> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp
> index 37d133cc..6f075560 100644
> --- a/src/libcamera/base/object.cpp
> +++ b/src/libcamera/base/object.cpp
> @@ -207,6 +207,9 @@ void Object::message(Message *msg)
>                  * it in release mode (with -O2 or -O3).
>                  */
>                 InvokeMessage *iMsg = dynamic_cast<InvokeMessage *>(msg);
> +                if(!iMsg)
> +                        break;
> +
>                 Semaphore *semaphore = iMsg->semaphore();
>                 iMsg->invoke();
>  
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> index 0537f785..86698684 100644
> --- a/src/libcamera/ipa_data_serializer.cpp
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -311,9 +311,11 @@ template<>
>  std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>  IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerializer *cs)
>  {
> -       if (!cs)
> +       if (!cs) {
>                 LOG(IPADataSerializer, Fatal)
>                         << "ControlSerializer not provided for serialization of ControlList";
> +               return {};
> +        }

Interesting one.

Fatal is supposed to be a non-return function, so the static analyser
probably doesn't realise that.

So return {}; is unreachable code here.


>  
>         size_t size;
>         std::vector<uint8_t> infoData;
> @@ -361,9 +363,11 @@ IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator
>                                             std::vector<uint8_t>::const_iterator dataEnd,
>                                             ControlSerializer *cs)
>  {
> -       if (!cs)
> +       if (!cs) {
>                 LOG(IPADataSerializer, Fatal)
>                         << "ControlSerializer not provided for deserialization of ControlList";
> +               return {};
> +        }
>  
>         if (std::distance(dataBegin, dataEnd) < 8)
>                 return {};
> @@ -436,9 +440,11 @@ std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>  IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map,
>                                              ControlSerializer *cs)
>  {
> -       if (!cs)
> +       if (!cs) {
>                 LOG(IPADataSerializer, Fatal)
>                         << "ControlSerializer not provided for serialization of ControlInfoMap";
> +               return {};
> +        }
>  
>         size_t size = cs->binarySize(map);
>         std::vector<uint8_t> infoData(size);
> @@ -463,9 +469,11 @@ IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_itera
>                                                std::vector<uint8_t>::const_iterator dataEnd,
>                                                ControlSerializer *cs)
>  {
> -       if (!cs)
> +       if (!cs) {
>                 LOG(IPADataSerializer, Fatal)
>                         << "ControlSerializer not provided for deserialization of ControlInfoMap";
> +               return {};
> +        }
>  
>         if (std::distance(dataBegin, dataEnd) < 4)
>                 return {};
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bf4c2921..15f918cd 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -565,7 +565,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  
>         /* Apply the format to the configured streams output devices. */
>         StreamConfiguration *mainCfg = nullptr;
> -       StreamConfiguration *vfCfg = nullptr;
>  
>         for (unsigned int i = 0; i < config->size(); ++i) {
>                 StreamConfiguration &cfg = (*config)[i];
> @@ -577,7 +576,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>                         if (ret)
>                                 return ret;
>                 } else if (stream == vfStream) {
> -                       vfCfg = &cfg;
>                         ret = imgu->configureViewfinder(cfg, &outputFormat);
>                         if (ret)
>                                 return ret;
> @@ -589,7 +587,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>          * the configuration of the active one for that purpose (there should
>          * be at least one active stream in the configuration request).
>          */
> -       if (!vfCfg) {
> +       if (mainCfg) {
>                 ret = imgu->configureViewfinder(*mainCfg, &outputFormat);
>                 if (ret)
>                         return ret;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index c6fe12d6..3a5f9095 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -985,6 +985,9 @@ void SimpleCameraData::clearIncompleteRequests()
>  
>  void SimpleCameraData::tryCompleteRequest(Request *request)
>  {
> +        if (!request)
> +                return;
> +

Can this ever happen? Does the caller for tryCompleteRequest already
guarantee request is a valid object?

>         if (request->hasPendingBuffers())
>                 return;
>  
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index d9f5b326..1b89a721 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -820,8 +820,6 @@ void DebayerCpuThread::process2(uint32_t frame, const uint8_t *src, uint8_t *dst
>                 /* next line may point outside of src, use prev. */
>                 linePointers[2] = linePointers[0];
>                 debayer_->debayer1(dst, linePointers);
> -               src += inputStride;
> -               dst += outputStride;
>         }
>  }
>  
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index e8b3eb9c..105f5bed 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -291,15 +291,16 @@ std::string V4L2PixelFormat::toString() const
>         char ss[8] = { static_cast<char>(fourcc_ & 0x7f),
>                        static_cast<char>((fourcc_ >> 8) & 0x7f),
>                        static_cast<char>((fourcc_ >> 16) & 0x7f),
> -                      static_cast<char>((fourcc_ >> 24) & 0x7f) };
> +                      static_cast<char>((fourcc_ >> 24) & 0x7f),
> +                       '-', 'B', 'E', 0 };
>  
>         for (unsigned int i = 0; i < 4; i++) {
>                 if (!isprint(ss[i]))
>                         ss[i] = '.';
>         }
>  
> -       if (fourcc_ & (1 << 31))
> -               strcat(ss, "-BE");
> +       if (!(fourcc_ & (1 << 31)))
> +               ss[4] = 0;

Interesting, but this shows why each independent change needs to be
split. This would need a write up in the commit message to justify and
explain why this change is better or more efficient. (I do like this
inversion trick though, especially as we've already allcoated the space
on the stack above anyway..., I'm not yet sure if one way is more
efficient than the other though ?)


--
Regards

Kieran

>  
>         return ss;
>  }
> -- 
> 2.43.0
>

Hi Neil,

On Thu, May 07, 2026 at 12:23:16PM +0100, Neil Matthews wrote:
> Dear All,
> First submission -- apologies if not following established procedures.

It's nearly right, pretty good for a first submission.

> Compiles, but untested.
> 
> There may be better solutions to the enum versus flag changes.

Have you looked at the Flags class
(https://libcamera.org/api-html/classlibcamera_1_1Flags.html) ?

> From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001
> From: Neil Matthews <drneildmatthews@googlemail.com>
> Date: Fri, 1 May 2026 16:46:30 +0100
> Subject: [PATCH] Fix some issues highlighted by static checking.

First comment: the patch should be sent as-in, not copied and pasted to
a mail after other text. The patch subject should be the subject of the
e-mail. I recommend using git-send-email, see
https://libcamera.org/contributing.html#submitting-patches for more
information.

> Checkers were invoked as:
> 
> cppcheck --enable=all --force --inconclusive
> 
> and as
> 
> bear ninja -C build
> find . -name "*.cpp" -exec clang-tidy {} ';'
> 
> Issues raised include:
> - using enums as flags that are or'ed together creating values not defined in the enum
> - checking result of malloc
> - checking (null) pointers before dereferencing
> - avoiding strcat()

The base rule is "one logical change per patch", so these four
categories of problems should be split to four separate patches (which
can be sent individually or as a series if they depend on each other).

> 
> Other flagged issues remain to be reviewed and fixed if required.
> 
> Signed-off-by: Neil Matthews <drneildmatthews@googlemail.com>
> ---
>  include/libcamera/base/memfd.h                 |  1 +
>  include/libcamera/controls.h                   |  1 +
>  include/libcamera/internal/dma_buf_allocator.h |  2 ++
>  src/android/metadata/camera_metadata.c         |  4 ++++
>  src/libcamera/base/object.cpp                  |  3 +++
>  src/libcamera/ipa_data_serializer.cpp          | 16 ++++++++++++----
>  src/libcamera/pipeline/ipu3/ipu3.cpp           |  4 +---
>  src/libcamera/pipeline/simple/simple.cpp       |  3 +++
>  src/libcamera/software_isp/debayer_cpu.cpp     |  2 --
>  src/libcamera/v4l2_pixelformat.cpp             |  7 ++++---
>  10 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h
> index 5ffa0f22..6805d374 100644
> --- a/include/libcamera/base/memfd.h
> +++ b/include/libcamera/base/memfd.h
> @@ -21,6 +21,7 @@ public:
>  		None = 0,
>  		Shrink = (1 << 0),
>  		Grow = (1 << 1),
> +                ShrinkAndGrow = Shrink | Grow,

This is using spaces instead of tabs for indentation. I don't know if
the issue was in your code, or caused by your mail client. This is
another reason why git-send-email is recommended, it will not mess up
with formating.

>  	};
>  
>  	using Seals = Flags<Seal>;
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 44ff4964..a22fe28f 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -253,6 +253,7 @@ public:
>  	enum class Direction {
>  		In = (1 << 0),
>  		Out = (1 << 1),
> +                InAndOut = In | Out,
>  	};
>  
>  	using DirectionFlags = Flags<Direction>;
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index 13600915..40dc36da 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -26,7 +26,9 @@ public:
>  	enum class DmaBufAllocatorFlag {
>  		CmaHeap = 1 << 0,
>  		SystemHeap = 1 << 1,
> +		CmaHeapAndSystemHeap = CmaHeap | SystemHeap,
>  		UDmaBuf = 1 << 2,
> +		CmaHeapAndSystemHeapAndUDmaBuf = CmaHeap | SystemHeap | UDmaBuf,

I don't think we want this kind of changes. It would require declaring
enumerators for all combinations of flags, which won't scale. I don't
see how it brings any practical improvement.

>  	};
>  
>  	using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>;
> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> index b86586a7..89cc792f 100644
> --- a/src/android/metadata/camera_metadata.c
> +++ b/src/android/metadata/camera_metadata.c
> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(
>      }
>  
>      void *buffer = malloc(src_size);
> +    if(!buffer) {
> +        return NULL;
> +    }

Please run checkstyle.py on your commits, it will check for most coding
style issues. See https://libcamera.org/coding-style.html#tools for more
information.

This file is part of the Android camera metadata library. We carry a
local copy, but we don't otherwise maintain it and I'd like to avoid
diverging from upstream. If you think this is important to fix, the fix
should be submitted to Android AOSP, and we can then update to a newer
version.

> +
>      memcpy(buffer, src, src_size);
>  
>      camera_metadata_t *metadata = (camera_metadata_t*) buffer;
> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp
> index 37d133cc..6f075560 100644
> --- a/src/libcamera/base/object.cpp
> +++ b/src/libcamera/base/object.cpp
> @@ -207,6 +207,9 @@ void Object::message(Message *msg)
>  		 * it in release mode (with -O2 or -O3).
>  		 */
>  		InvokeMessage *iMsg = dynamic_cast<InvokeMessage *>(msg);
> +                if(!iMsg)
> +                        break;

As the comment above mentions, this is to work around a compiler issue.
I don't think a runtime check is needed.

> +
>  		Semaphore *semaphore = iMsg->semaphore();
>  		iMsg->invoke();
>  
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> index 0537f785..86698684 100644
> --- a/src/libcamera/ipa_data_serializer.cpp
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -311,9 +311,11 @@ template<>
>  std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>  IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerializer *cs)
>  {
> -	if (!cs)
> +	if (!cs) {
>  		LOG(IPADataSerializer, Fatal)
>  			<< "ControlSerializer not provided for serialization of ControlList";
> +		return {};

The commit message does not mention these changes, could you explain why
you think they're needed ?

> +        }
>  
>  	size_t size;
>  	std::vector<uint8_t> infoData;
> @@ -361,9 +363,11 @@ IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator
>  					    std::vector<uint8_t>::const_iterator dataEnd,
>  					    ControlSerializer *cs)
>  {
> -	if (!cs)
> +	if (!cs) {
>  		LOG(IPADataSerializer, Fatal)
>  			<< "ControlSerializer not provided for deserialization of ControlList";
> +		return {};
> +        }
>  
>  	if (std::distance(dataBegin, dataEnd) < 8)
>  		return {};
> @@ -436,9 +440,11 @@ std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>  IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map,
>  					     ControlSerializer *cs)
>  {
> -	if (!cs)
> +	if (!cs) {
>  		LOG(IPADataSerializer, Fatal)
>  			<< "ControlSerializer not provided for serialization of ControlInfoMap";
> +		return {};
> +        }
>  
>  	size_t size = cs->binarySize(map);
>  	std::vector<uint8_t> infoData(size);
> @@ -463,9 +469,11 @@ IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_itera
>  					       std::vector<uint8_t>::const_iterator dataEnd,
>  					       ControlSerializer *cs)
>  {
> -	if (!cs)
> +	if (!cs) {
>  		LOG(IPADataSerializer, Fatal)
>  			<< "ControlSerializer not provided for deserialization of ControlInfoMap";
> +		return {};
> +        }
>  
>  	if (std::distance(dataBegin, dataEnd) < 4)
>  		return {};
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bf4c2921..15f918cd 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -565,7 +565,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  
>  	/* Apply the format to the configured streams output devices. */
>  	StreamConfiguration *mainCfg = nullptr;
> -	StreamConfiguration *vfCfg = nullptr;
>  
>  	for (unsigned int i = 0; i < config->size(); ++i) {
>  		StreamConfiguration &cfg = (*config)[i];
> @@ -577,7 +576,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  			if (ret)
>  				return ret;
>  		} else if (stream == vfStream) {
> -			vfCfg = &cfg;
>  			ret = imgu->configureViewfinder(cfg, &outputFormat);
>  			if (ret)
>  				return ret;
> @@ -589,7 +587,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	 * the configuration of the active one for that purpose (there should
>  	 * be at least one active stream in the configuration request).
>  	 */
> -	if (!vfCfg) {
> +	if (mainCfg) {
>  		ret = imgu->configureViewfinder(*mainCfg, &outputFormat);
>  		if (ret)
>  			return ret;

Why ?

> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index c6fe12d6..3a5f9095 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -985,6 +985,9 @@ void SimpleCameraData::clearIncompleteRequests()
>  
>  void SimpleCameraData::tryCompleteRequest(Request *request)
>  {
> +        if (!request)
> +                return;
> +

What makes you think the function can be called with a null request
pointer ?

>  	if (request->hasPendingBuffers())
>  		return;
>  
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index d9f5b326..1b89a721 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -820,8 +820,6 @@ void DebayerCpuThread::process2(uint32_t frame, const uint8_t *src, uint8_t *dst
>  		/* next line may point outside of src, use prev. */
>  		linePointers[2] = linePointers[0];
>  		debayer_->debayer1(dst, linePointers);
> -		src += inputStride;
> -		dst += outputStride;

This could bring a small performance improvement. It should be split to
a separate patch, with a commit message to explain it.

>  	}
>  }
>  
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index e8b3eb9c..105f5bed 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -291,15 +291,16 @@ std::string V4L2PixelFormat::toString() const
>  	char ss[8] = { static_cast<char>(fourcc_ & 0x7f),
>  		       static_cast<char>((fourcc_ >> 8) & 0x7f),
>  		       static_cast<char>((fourcc_ >> 16) & 0x7f),
> -		       static_cast<char>((fourcc_ >> 24) & 0x7f) };
> +		       static_cast<char>((fourcc_ >> 24) & 0x7f),
> +                       '-', 'B', 'E', 0 };
>  
>  	for (unsigned int i = 0; i < 4; i++) {
>  		if (!isprint(ss[i]))
>  			ss[i] = '.';
>  	}
>  
> -	if (fourcc_ & (1 << 31))
> -		strcat(ss, "-BE");
> +	if (!(fourcc_ & (1 << 31)))
> +		ss[4] = 0;

This should also be a separate patch, with a clear commit message.

>  
>  	return ss;
>  }

Hi

2026. 05. 07. 13:23 keltezéssel, Neil Matthews írta:
> Dear All,
> First submission -- apologies if not following established procedures.
> Compiles, but untested.
> 
> There may be better solutions to the enum versus flag changes.
> 
> Best regards,
> Neil Matthews
> 
>  From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001
> From: Neil Matthews <drneildmatthews@googlemail.com>
> Date: Fri, 1 May 2026 16:46:30 +0100
> Subject: [PATCH] Fix some issues highlighted by static checking.
> 
> Checkers were invoked as:
> 
> cppcheck --enable=all --force --inconclusive
> 
> and as
> 
> bear ninja -C build
> find . -name "*.cpp" -exec clang-tidy {} ';'
> 
> Issues raised include:
> - using enums as flags that are or'ed together creating values not defined in the enum
> - checking result of malloc
> - checking (null) pointers before dereferencing
> - avoiding strcat()
> 
> Other flagged issues remain to be reviewed and fixed if required.
> 
> Signed-off-by: Neil Matthews <drneildmatthews@googlemail.com>
> ---
>   include/libcamera/base/memfd.h                 |  1 +
>   include/libcamera/controls.h                   |  1 +
>   include/libcamera/internal/dma_buf_allocator.h |  2 ++
>   src/android/metadata/camera_metadata.c         |  4 ++++
>   src/libcamera/base/object.cpp                  |  3 +++
>   src/libcamera/ipa_data_serializer.cpp          | 16 ++++++++++++----
>   src/libcamera/pipeline/ipu3/ipu3.cpp           |  4 +---
>   src/libcamera/pipeline/simple/simple.cpp       |  3 +++
>   src/libcamera/software_isp/debayer_cpu.cpp     |  2 --
>   src/libcamera/v4l2_pixelformat.cpp             |  7 ++++---
>   10 files changed, 31 insertions(+), 12 deletions(-)
> 
> [...]
> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> index b86586a7..89cc792f 100644
> --- a/src/android/metadata/camera_metadata.c
> +++ b/src/android/metadata/camera_metadata.c
> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(
>       }
>   
>       void *buffer = malloc(src_size);
> +    if(!buffer) {
> +        return NULL;
> +    }

This file is essentially imported from android:
https://android.googlesource.com/platform/system/media/+/master/camera/src/camera_metadata.c
so I think local modification should be avoided if possible.


> +
>       memcpy(buffer, src, src_size);
>   
>       camera_metadata_t *metadata = (camera_metadata_t*) buffer;
> [...]