[{"id":38766,"web_url":"https://patchwork.libcamera.org/comment/38766/","msgid":"<177815453098.33696.15711661135250731095@ping.linuxembedded.co.uk>","date":"2026-05-07T11:48:50","subject":"Re: Fix some issues highlighted by static checking","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Neil,\n\nQuoting Neil Matthews (2026-05-07 12:23:16)\n> Dear All,\n> First submission -- apologies if not following established procedures.\n> Compiles, but untested.\n> \n> There may be better solutions to the enum versus flag changes.\n> \n> Best regards,\n> Neil Matthews\n> \n> From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001\n> From: Neil Matthews \n> Date: Fri, 1 May 2026 16:46:30 +0100\n> Subject: [PATCH] Fix some issues highlighted by static checking.\n> \n> Checkers were invoked as:\n> \n> cppcheck --enable=all --force --inconclusive\n> \n> and as\n\nThank you for the cleanups.\n\nEach one is likely suitable to be a single distinct patch with a clear\ncommit message to explain the issue and the update in the patch.\n\nPlease see: \n\nAnd I always recommend https://cbea.ms/git-commit/ as a great read for\nhow we like to structure and document commits 'atomically' such that\neach commit performs a single purpose.\n\n> \n> bear ninja -C build\n> find . -name \"*.cpp\" -exec clang-tidy {} ';'\n> \n> Issues raised include:\n> - using enums as flags that are or'ed together creating values not defined in the enum\n\n> - checking result of malloc\n> - checking (null) pointers before dereferencing\n> - avoiding strcat()\n\n> \n> Other flagged issues remain to be reviewed and fixed if required.\n> \n> Signed-off-by: Neil Matthews \n> ---\n> include/libcamera/base/memfd.h | 1 +\n> include/libcamera/controls.h | 1 +\n> include/libcamera/internal/dma_buf_allocator.h | 2 ++\n> src/android/metadata/camera_metadata.c | 4 ++++\n> src/libcamera/base/object.cpp | 3 +++\n> src/libcamera/ipa_data_serializer.cpp | 16 ++++++++++++----\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +---\n> src/libcamera/pipeline/simple/simple.cpp | 3 +++\n> src/libcamera/software_isp/debayer_cpu.cpp | 2 --\n> src/libcamera/v4l2_pixelformat.cpp | 7 ++++---\n> 10 files changed, 31 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h\n> index 5ffa0f22..6805d374 100644\n> --- a/include/libcamera/base/memfd.h\n> +++ b/include/libcamera/base/memfd.h\n> @@ -21,6 +21,7 @@ public:\n> None = 0,\n> Shrink = (1 << 0),\n> Grow = (1 << 1),\n> + ShrinkAndGrow = Shrink | Grow,\n> };\n\nI'm not sure if these sort of detract from the purpose of using bit\nfields here.\n\nWith this we'd have to make every combintion of bits in the enum?\n\n> \n> using Seals = Flags;\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 44ff4964..a22fe28f 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -253,6 +253,7 @@ public:\n> enum class Direction {\n> In = (1 << 0),\n> Out = (1 << 1),\n> + InAndOut = In | Out,\n> };\n> \n> using DirectionFlags = Flags;\n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index 13600915..40dc36da 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -26,7 +26,9 @@ public:\n> enum class DmaBufAllocatorFlag {\n> CmaHeap = 1 << 0,\n> SystemHeap = 1 << 1,\n> + CmaHeapAndSystemHeap = CmaHeap | SystemHeap,\n> UDmaBuf = 1 << 2,\n> + CmaHeapAndSystemHeapAndUDmaBuf = CmaHeap | SystemHeap | UDmaBuf,\n> };\n> \n> using DmaBufAllocatorFlags = Flags;\n> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> index b86586a7..89cc792f 100644\n> --- a/src/android/metadata/camera_metadata.c\n> +++ b/src/android/metadata/camera_metadata.c\n> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(\n> }\n> \n> void *buffer = malloc(src_size);\n> + if(!buffer) {\n> + return NULL;\n> + }\n> +\n\nThis would just be:\n\n\tif (!buffer)\n\t\treturn NULL;\n\nin our coding style, but I think in general we make the assumption\nmalloc can't fail? (I think I recall that just like we assume 'new' in\nC++ always succeeds).\n\n\n\n> memcpy(buffer, src, src_size);\n> \n> camera_metadata_t *metadata = (camera_metadata_t*) buffer;\n> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> index 37d133cc..6f075560 100644\n> --- a/src/libcamera/base/object.cpp\n> +++ b/src/libcamera/base/object.cpp\n> @@ -207,6 +207,9 @@ void Object::message(Message *msg)\n> * it in release mode (with -O2 or -O3).\n> */\n> InvokeMessage *iMsg = dynamic_cast(msg);\n> + if(!iMsg)\n> + break;\n> +\n> Semaphore *semaphore = iMsg->semaphore();\n> iMsg->invoke();\n> \n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index 0537f785..86698684 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -311,9 +311,11 @@ template<>\n> std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs)\n> {\n> - if (!cs)\n> + if (!cs) {\n> LOG(IPADataSerializer, Fatal)\n> << \"ControlSerializer not provided for serialization of ControlList\";\n> + return {};\n> + }\n\nInteresting one.\n\nFatal is supposed to be a non-return function, so the static analyser\nprobably doesn't realise that.\n\nSo return {}; is unreachable code here.\n\n\n> \n> size_t size;\n> std::vector infoData;\n> @@ -361,9 +363,11 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> std::vector::const_iterator dataEnd,\n> ControlSerializer *cs)\n> {\n> - if (!cs)\n> + if (!cs) {\n> LOG(IPADataSerializer, Fatal)\n> << \"ControlSerializer not provided for deserialization of ControlList\";\n> + return {};\n> + }\n> \n> if (std::distance(dataBegin, dataEnd) < 8)\n> return {};\n> @@ -436,9 +440,11 @@ std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlInfoMap &map,\n> ControlSerializer *cs)\n> {\n> - if (!cs)\n> + if (!cs) {\n> LOG(IPADataSerializer, Fatal)\n> << \"ControlSerializer not provided for serialization of ControlInfoMap\";\n> + return {};\n> + }\n> \n> size_t size = cs->binarySize(map);\n> std::vector infoData(size);\n> @@ -463,9 +469,11 @@ IPADataSerializer::deserialize(std::vector::const_itera\n> std::vector::const_iterator dataEnd,\n> ControlSerializer *cs)\n> {\n> - if (!cs)\n> + if (!cs) {\n> LOG(IPADataSerializer, Fatal)\n> << \"ControlSerializer not provided for deserialization of ControlInfoMap\";\n> + return {};\n> + }\n> \n> if (std::distance(dataBegin, dataEnd) < 4)\n> return {};\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index bf4c2921..15f918cd 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -565,7 +565,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \n> /* Apply the format to the configured streams output devices. */\n> StreamConfiguration *mainCfg = nullptr;\n> - StreamConfiguration *vfCfg = nullptr;\n> \n> for (unsigned int i = 0; i < config->size(); ++i) {\n> StreamConfiguration &cfg = (*config)[i];\n> @@ -577,7 +576,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> if (ret)\n> return ret;\n> } else if (stream == vfStream) {\n> - vfCfg = &cfg;\n> ret = imgu->configureViewfinder(cfg, &outputFormat);\n> if (ret)\n> return ret;\n> @@ -589,7 +587,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> * the configuration of the active one for that purpose (there should\n> * be at least one active stream in the configuration request).\n> */\n> - if (!vfCfg) {\n> + if (mainCfg) {\n> ret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n> if (ret)\n> return ret;\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index c6fe12d6..3a5f9095 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -985,6 +985,9 @@ void SimpleCameraData::clearIncompleteRequests()\n> \n> void SimpleCameraData::tryCompleteRequest(Request *request)\n> {\n> + if (!request)\n> + return;\n> +\n\nCan this ever happen? Does the caller for tryCompleteRequest already\nguarantee request is a valid object?\n\n> if (request->hasPendingBuffers())\n> return;\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index d9f5b326..1b89a721 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -820,8 +820,6 @@ void DebayerCpuThread::process2(uint32_t frame, const uint8_t *src, uint8_t *dst\n> /* next line may point outside of src, use prev. */\n> linePointers[2] = linePointers[0];\n> debayer_->debayer1(dst, linePointers);\n> - src += inputStride;\n> - dst += outputStride;\n> }\n> }\n> \n> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> index e8b3eb9c..105f5bed 100644\n> --- a/src/libcamera/v4l2_pixelformat.cpp\n> +++ b/src/libcamera/v4l2_pixelformat.cpp\n> @@ -291,15 +291,16 @@ std::string V4L2PixelFormat::toString() const\n> char ss[8] = { static_cast(fourcc_ & 0x7f),\n> static_cast((fourcc_ >> 8) & 0x7f),\n> static_cast((fourcc_ >> 16) & 0x7f),\n> - static_cast((fourcc_ >> 24) & 0x7f) };\n> + static_cast((fourcc_ >> 24) & 0x7f),\n> + '-', 'B', 'E', 0 };\n> \n> for (unsigned int i = 0; i < 4; i++) {\n> if (!isprint(ss[i]))\n> ss[i] = '.';\n> }\n> \n> - if (fourcc_ & (1 << 31))\n> - strcat(ss, \"-BE\");\n> + if (!(fourcc_ & (1 << 31)))\n> + ss[4] = 0;\n\nInteresting, but this shows why each independent change needs to be\nsplit. This would need a write up in the commit message to justify and\nexplain why this change is better or more efficient. (I do like this\ninversion trick though, especially as we've already allcoated the space\non the stack above anyway..., I'm not yet sure if one way is more\nefficient than the other though ?)\n\n\n--\nRegards\n\nKieran\n\n> \n> return ss;\n> }\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C2490BDCB5\n\tfor ;\n\tThu, 7 May 2026 11:48:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FBF26301E;\n\tThu, 7 May 2026 13:48:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC83C62010\n\tfor ;\n\tThu, 7 May 2026 13:48:53 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C61699CE;\n\tThu, 7 May 2026 13:48:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K9XwDX7/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778154529;\n\tbh=jbkwnYScx9QCC0TeYwXdzu47wFLPui64bh6lZsCuAko=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=K9XwDX7/gB1AuxhFu3X5Y9I5CdhAo+0ELdcGxItchBvceGzbNqepZRWghdzn0cd4q\n\trUc0Bs3d1fYzIw9duCGzsQaEf2sbVZwXimHEF7H05sZDyS8Jo+NF0HM0Y+GrEx3c4K\n\tZcQ+ZZWtsS8PY70s3n6MZajw8c1B2MENyGb82JVA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","References":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","Subject":"Re: Fix some issues highlighted by static checking","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNeil Matthews ","To":"Neil Matthews ","Date":"Thu, 07 May 2026 12:48:50 +0100","Message-ID":"<177815453098.33696.15711661135250731095@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38767,"web_url":"https://patchwork.libcamera.org/comment/38767/","msgid":"<20260507120646.GB1938994@killaraus.ideasonboard.com>","date":"2026-05-07T12:06:46","subject":"Re: Fix some issues highlighted by static checking","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Neil,\n\nOn Thu, May 07, 2026 at 12:23:16PM +0100, Neil Matthews wrote:\n> Dear All,\n> First submission -- apologies if not following established procedures.\n\nIt's nearly right, pretty good for a first submission.\n\n> Compiles, but untested.\n> \n> There may be better solutions to the enum versus flag changes.\n\nHave you looked at the Flags class\n(https://libcamera.org/api-html/classlibcamera_1_1Flags.html) ?\n\n> From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001\n> From: Neil Matthews \n> Date: Fri, 1 May 2026 16:46:30 +0100\n> Subject: [PATCH] Fix some issues highlighted by static checking.\n\nFirst comment: the patch should be sent as-in, not copied and pasted to\na mail after other text. The patch subject should be the subject of the\ne-mail. I recommend using git-send-email, see\nhttps://libcamera.org/contributing.html#submitting-patches for more\ninformation.\n\n> Checkers were invoked as:\n> \n> cppcheck --enable=all --force --inconclusive\n> \n> and as\n> \n> bear ninja -C build\n> find . -name \"*.cpp\" -exec clang-tidy {} ';'\n> \n> Issues raised include:\n> - using enums as flags that are or'ed together creating values not defined in the enum\n> - checking result of malloc\n> - checking (null) pointers before dereferencing\n> - avoiding strcat()\n\nThe base rule is \"one logical change per patch\", so these four\ncategories of problems should be split to four separate patches (which\ncan be sent individually or as a series if they depend on each other).\n\n> \n> Other flagged issues remain to be reviewed and fixed if required.\n> \n> Signed-off-by: Neil Matthews \n> ---\n> include/libcamera/base/memfd.h | 1 +\n> include/libcamera/controls.h | 1 +\n> include/libcamera/internal/dma_buf_allocator.h | 2 ++\n> src/android/metadata/camera_metadata.c | 4 ++++\n> src/libcamera/base/object.cpp | 3 +++\n> src/libcamera/ipa_data_serializer.cpp | 16 ++++++++++++----\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +---\n> src/libcamera/pipeline/simple/simple.cpp | 3 +++\n> src/libcamera/software_isp/debayer_cpu.cpp | 2 --\n> src/libcamera/v4l2_pixelformat.cpp | 7 ++++---\n> 10 files changed, 31 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h\n> index 5ffa0f22..6805d374 100644\n> --- a/include/libcamera/base/memfd.h\n> +++ b/include/libcamera/base/memfd.h\n> @@ -21,6 +21,7 @@ public:\n> \t\tNone = 0,\n> \t\tShrink = (1 << 0),\n> \t\tGrow = (1 << 1),\n> + ShrinkAndGrow = Shrink | Grow,\n\nThis is using spaces instead of tabs for indentation. I don't know if\nthe issue was in your code, or caused by your mail client. This is\nanother reason why git-send-email is recommended, it will not mess up\nwith formating.\n\n> \t};\n> \n> \tusing Seals = Flags;\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 44ff4964..a22fe28f 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -253,6 +253,7 @@ public:\n> \tenum class Direction {\n> \t\tIn = (1 << 0),\n> \t\tOut = (1 << 1),\n> + InAndOut = In | Out,\n> \t};\n> \n> \tusing DirectionFlags = Flags;\n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index 13600915..40dc36da 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -26,7 +26,9 @@ public:\n> \tenum class DmaBufAllocatorFlag {\n> \t\tCmaHeap = 1 << 0,\n> \t\tSystemHeap = 1 << 1,\n> +\t\tCmaHeapAndSystemHeap = CmaHeap | SystemHeap,\n> \t\tUDmaBuf = 1 << 2,\n> +\t\tCmaHeapAndSystemHeapAndUDmaBuf = CmaHeap | SystemHeap | UDmaBuf,\n\nI don't think we want this kind of changes. It would require declaring\nenumerators for all combinations of flags, which won't scale. I don't\nsee how it brings any practical improvement.\n\n> \t};\n> \n> \tusing DmaBufAllocatorFlags = Flags;\n> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> index b86586a7..89cc792f 100644\n> --- a/src/android/metadata/camera_metadata.c\n> +++ b/src/android/metadata/camera_metadata.c\n> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(\n> }\n> \n> void *buffer = malloc(src_size);\n> + if(!buffer) {\n> + return NULL;\n> + }\n\nPlease run checkstyle.py on your commits, it will check for most coding\nstyle issues. See https://libcamera.org/coding-style.html#tools for more\ninformation.\n\nThis file is part of the Android camera metadata library. We carry a\nlocal copy, but we don't otherwise maintain it and I'd like to avoid\ndiverging from upstream. If you think this is important to fix, the fix\nshould be submitted to Android AOSP, and we can then update to a newer\nversion.\n\n> +\n> memcpy(buffer, src, src_size);\n> \n> camera_metadata_t *metadata = (camera_metadata_t*) buffer;\n> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> index 37d133cc..6f075560 100644\n> --- a/src/libcamera/base/object.cpp\n> +++ b/src/libcamera/base/object.cpp\n> @@ -207,6 +207,9 @@ void Object::message(Message *msg)\n> \t\t * it in release mode (with -O2 or -O3).\n> \t\t */\n> \t\tInvokeMessage *iMsg = dynamic_cast(msg);\n> + if(!iMsg)\n> + break;\n\nAs the comment above mentions, this is to work around a compiler issue.\nI don't think a runtime check is needed.\n\n> +\n> \t\tSemaphore *semaphore = iMsg->semaphore();\n> \t\tiMsg->invoke();\n> \n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index 0537f785..86698684 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -311,9 +311,11 @@ template<>\n> std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs)\n> {\n> -\tif (!cs)\n> +\tif (!cs) {\n> \t\tLOG(IPADataSerializer, Fatal)\n> \t\t\t<< \"ControlSerializer not provided for serialization of ControlList\";\n> +\t\treturn {};\n\nThe commit message does not mention these changes, could you explain why\nyou think they're needed ?\n\n> + }\n> \n> \tsize_t size;\n> \tstd::vector infoData;\n> @@ -361,9 +363,11 @@ IPADataSerializer::deserialize(std::vector::const_iterator\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> -\tif (!cs)\n> +\tif (!cs) {\n> \t\tLOG(IPADataSerializer, Fatal)\n> \t\t\t<< \"ControlSerializer not provided for deserialization of ControlList\";\n> +\t\treturn {};\n> + }\n> \n> \tif (std::distance(dataBegin, dataEnd) < 8)\n> \t\treturn {};\n> @@ -436,9 +440,11 @@ std::tuple, std::vector>\n> IPADataSerializer::serialize(const ControlInfoMap &map,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> -\tif (!cs)\n> +\tif (!cs) {\n> \t\tLOG(IPADataSerializer, Fatal)\n> \t\t\t<< \"ControlSerializer not provided for serialization of ControlInfoMap\";\n> +\t\treturn {};\n> + }\n> \n> \tsize_t size = cs->binarySize(map);\n> \tstd::vector infoData(size);\n> @@ -463,9 +469,11 @@ IPADataSerializer::deserialize(std::vector::const_itera\n> \t\t\t\t\t std::vector::const_iterator dataEnd,\n> \t\t\t\t\t ControlSerializer *cs)\n> {\n> -\tif (!cs)\n> +\tif (!cs) {\n> \t\tLOG(IPADataSerializer, Fatal)\n> \t\t\t<< \"ControlSerializer not provided for deserialization of ControlInfoMap\";\n> +\t\treturn {};\n> + }\n> \n> \tif (std::distance(dataBegin, dataEnd) < 4)\n> \t\treturn {};\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index bf4c2921..15f918cd 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -565,7 +565,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \n> \t/* Apply the format to the configured streams output devices. */\n> \tStreamConfiguration *mainCfg = nullptr;\n> -\tStreamConfiguration *vfCfg = nullptr;\n> \n> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n> \t\tStreamConfiguration &cfg = (*config)[i];\n> @@ -577,7 +576,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \t\t} else if (stream == vfStream) {\n> -\t\t\tvfCfg = &cfg;\n> \t\t\tret = imgu->configureViewfinder(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> @@ -589,7 +587,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * the configuration of the active one for that purpose (there should\n> \t * be at least one active stream in the configuration request).\n> \t */\n> -\tif (!vfCfg) {\n> +\tif (mainCfg) {\n> \t\tret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n\nWhy ?\n\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index c6fe12d6..3a5f9095 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -985,6 +985,9 @@ void SimpleCameraData::clearIncompleteRequests()\n> \n> void SimpleCameraData::tryCompleteRequest(Request *request)\n> {\n> + if (!request)\n> + return;\n> +\n\nWhat makes you think the function can be called with a null request\npointer ?\n\n> \tif (request->hasPendingBuffers())\n> \t\treturn;\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index d9f5b326..1b89a721 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -820,8 +820,6 @@ void DebayerCpuThread::process2(uint32_t frame, const uint8_t *src, uint8_t *dst\n> \t\t/* next line may point outside of src, use prev. */\n> \t\tlinePointers[2] = linePointers[0];\n> \t\tdebayer_->debayer1(dst, linePointers);\n> -\t\tsrc += inputStride;\n> -\t\tdst += outputStride;\n\nThis could bring a small performance improvement. It should be split to\na separate patch, with a commit message to explain it.\n\n> \t}\n> }\n> \n> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> index e8b3eb9c..105f5bed 100644\n> --- a/src/libcamera/v4l2_pixelformat.cpp\n> +++ b/src/libcamera/v4l2_pixelformat.cpp\n> @@ -291,15 +291,16 @@ std::string V4L2PixelFormat::toString() const\n> \tchar ss[8] = { static_cast(fourcc_ & 0x7f),\n> \t\t static_cast((fourcc_ >> 8) & 0x7f),\n> \t\t static_cast((fourcc_ >> 16) & 0x7f),\n> -\t\t static_cast((fourcc_ >> 24) & 0x7f) };\n> +\t\t static_cast((fourcc_ >> 24) & 0x7f),\n> + '-', 'B', 'E', 0 };\n> \n> \tfor (unsigned int i = 0; i < 4; i++) {\n> \t\tif (!isprint(ss[i]))\n> \t\t\tss[i] = '.';\n> \t}\n> \n> -\tif (fourcc_ & (1 << 31))\n> -\t\tstrcat(ss, \"-BE\");\n> +\tif (!(fourcc_ & (1 << 31)))\n> +\t\tss[4] = 0;\n\nThis should also be a separate patch, with a clear commit message.\n\n> \n> \treturn ss;\n> }","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DAB82BE173\n\tfor ;\n\tThu, 7 May 2026 12:06:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BE8E762FE1;\n\tThu, 7 May 2026 14:06:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4885062010\n\tfor ;\n\tThu, 7 May 2026 14:06:48 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EAF89664;\n\tThu, 7 May 2026 14:06:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PEAfTMr2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778155604;\n\tbh=1yoz17Hw6T/PAIJ18CdW7zq10zxDL7VXl8hd5MShXTk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PEAfTMr2Nx5mOgoyDJ0sXCXH7Xrxl16WsaFXxVpmkZahDMfsGoYvHTct40WeZHL+W\n\t6it5YYPRQ0iXJOV0tnAtAN+FYcH90+3lIikS/NCXNQrcpG9nwr60kIHcpXziydzIij\n\teWiNDmNJKav8F3cFsOKytYPZ3d1VxM5uH9xVw4MU=","Date":"Thu, 7 May 2026 15:06:46 +0300","From":"Laurent Pinchart ","To":"Neil Matthews ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: Fix some issues highlighted by static checking","Message-ID":"<20260507120646.GB1938994@killaraus.ideasonboard.com>","References":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38769,"web_url":"https://patchwork.libcamera.org/comment/38769/","msgid":"<23a5ed24-116c-41cc-8494-27112f252952@ideasonboard.com>","date":"2026-05-07T12:25:25","subject":"Re: Fix some issues highlighted by static checking","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 05. 07. 13:23 keltezéssel, Neil Matthews írta:\n> Dear All,\n> First submission -- apologies if not following established procedures.\n> Compiles, but untested.\n> \n> There may be better solutions to the enum versus flag changes.\n> \n> Best regards,\n> Neil Matthews\n> \n> From a1f071f276612e97ab29c27dc6fb5bf62d0599f0 Mon Sep 17 00:00:00 2001\n> From: Neil Matthews \n> Date: Fri, 1 May 2026 16:46:30 +0100\n> Subject: [PATCH] Fix some issues highlighted by static checking.\n> \n> Checkers were invoked as:\n> \n> cppcheck --enable=all --force --inconclusive\n> \n> and as\n> \n> bear ninja -C build\n> find . -name \"*.cpp\" -exec clang-tidy {} ';'\n> \n> Issues raised include:\n> - using enums as flags that are or'ed together creating values not defined in the enum\n> - checking result of malloc\n> - checking (null) pointers before dereferencing\n> - avoiding strcat()\n> \n> Other flagged issues remain to be reviewed and fixed if required.\n> \n> Signed-off-by: Neil Matthews \n> ---\n> include/libcamera/base/memfd.h | 1 +\n> include/libcamera/controls.h | 1 +\n> include/libcamera/internal/dma_buf_allocator.h | 2 ++\n> src/android/metadata/camera_metadata.c | 4 ++++\n> src/libcamera/base/object.cpp | 3 +++\n> src/libcamera/ipa_data_serializer.cpp | 16 ++++++++++++----\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +---\n> src/libcamera/pipeline/simple/simple.cpp | 3 +++\n> src/libcamera/software_isp/debayer_cpu.cpp | 2 --\n> src/libcamera/v4l2_pixelformat.cpp | 7 ++++---\n> 10 files changed, 31 insertions(+), 12 deletions(-)\n> \n> [...]\n> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> index b86586a7..89cc792f 100644\n> --- a/src/android/metadata/camera_metadata.c\n> +++ b/src/android/metadata/camera_metadata.c\n> @@ -242,6 +242,10 @@ camera_metadata_t *allocate_copy_camera_metadata_checked(\n> }\n> \n> void *buffer = malloc(src_size);\n> + if(!buffer) {\n> + return NULL;\n> + }\n\nThis file is essentially imported from android:\nhttps://android.googlesource.com/platform/system/media/+/master/camera/src/camera_metadata.c\nso I think local modification should be avoided if possible.\n\n\n> +\n> memcpy(buffer, src, src_size);\n> \n> camera_metadata_t *metadata = (camera_metadata_t*) buffer;\n> [...]","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 27FF2BDCB5\n\tfor ;\n\tThu, 7 May 2026 12:25:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECADD62010;\n\tThu, 7 May 2026 14:25:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 122C862010\n\tfor ;\n\tThu, 7 May 2026 14:25:30 +0200 (CEST)","from [192.168.33.83] (185.221.140.217.nat.pool.zt.hu\n\t[185.221.140.217])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7188664;\n\tThu, 7 May 2026 14:25:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TXZXZ2Ji\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778156726;\n\tbh=jmq6ZMTsSzCh77Bdz/wimnsZTYRI0dsaZ4C6KHUS6FA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TXZXZ2JiBYIxzC6cl0SITAhEIF4+uWJPpxA0pOBDUWnYYvDp22GBShBeEgx1uFiH5\n\tcKxBBZwp3B+b91wm6Gnpx8VHwui2YJpSPbkpAOFXii+7Zpi6lKvJzn6DufEEjzIcST\n\t03CI/NWvMvUpGxnDfB/g/q+Vtmq40merdDOfi3eU=","Message-ID":"<23a5ed24-116c-41cc-8494-27112f252952@ideasonboard.com>","Date":"Thu, 7 May 2026 14:25:25 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: Fix some issues highlighted by static checking","To":"Neil Matthews ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260507112321.22726-1-drneildmatthews@googlemail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]