[{"id":13615,"web_url":"https://patchwork.libcamera.org/comment/13615/","msgid":"<20201106001454.GI2008567@oden.dyn.berto.se>","date":"2020-11-06T00:14:54","subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: v4l2_videodevice:\n\tZero-initialize planes in V4L2DeviceFormat","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-11-04 09:48:39 +0200, Laurent Pinchart wrote:\n> The V4L2DeviceFormat class doesn't have a default constructor, neither\n> does it specifies default member initializers for the plane-related\n> members. This results in the planes array and planesCount members being\n> uninitialized by default, leading to undefined behaviour if the user of\n> the class doesn't initialize it explicitly.\n> \n> Most users initialize V4L2DeviceFormat instances, but some don't. We\n> could fix them, but that would likely turn into a game of whack-a-mole.\n> As there's no use case for instantiating a large number of\n> V4L2DeviceFormat instances in a performance-critical code path, let's\n> instead add default initializers to avoid future issues.\n> \n> While at it, define a type of the structures containing plane\n> information, and use an std::array.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> include/libcamera/internal/v4l2_videodevice.h | 13 ++++++++-----\n> src/libcamera/v4l2_videodevice.cpp | 9 +++++++++\n> 2 files changed, 17 insertions(+), 5 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 53f6a2d5515b..dcb9654a34b8 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__\n> #define __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -153,14 +154,16 @@ private:\n> class V4L2DeviceFormat\n> {\n> public:\n> +\tstruct Plane {\n> +\t\tuint32_t size = 0;\n> +\t\tuint32_t bpl = 0;\n> +\t};\n> +\n> \tV4L2PixelFormat fourcc;\n> \tSize size;\n> \n> -\tstruct {\n> -\t\tuint32_t size;\n> -\t\tuint32_t bpl;\n> -\t} planes[3];\n> -\tunsigned int planesCount;\n> +\tstd::array planes;\n> +\tunsigned int planesCount = 0;\n> \n> \tconst std::string toString() const;\n> };\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 36d7d9a0f27a..d07c3530eea5 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -351,6 +351,15 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const\n> * V4L2DeviceFormat::planesCount value.\n> */\n> \n> +/**\n> + * \\struct V4L2DeviceFormat::Plane\n> + * \\brief Per-plane memory size information\n> + * \\var V4L2DeviceFormat::Plane::size\n> + * \\brief The plane total memory size (in bytes)\n> + * \\var V4L2DeviceFormat::Plane::bpl\n> + * \\brief The plane line stride (in bytes)\n> + */\n> +\n> /**\n> * \\var V4L2DeviceFormat::size\n> * \\brief The image size in pixels\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 F214BBE082\n\tfor ;\n\tFri, 6 Nov 2020 00:15:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 727E962D0E;\n\tFri, 6 Nov 2020 01:15:00 +0100 (CET)","from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B393C62CAF\n\tfor ;\n\tFri, 6 Nov 2020 01:14:57 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id v19so3475673lji.5\n\tfor ;\n\tThu, 05 Nov 2020 16:14:57 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw1sm333465lfe.162.2020.11.05.16.14.55\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 05 Nov 2020 16:14:55 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"UDEAgqJz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=7+GCUJI05LqWCHEsqvMhHA6ZZGztXnC5zQBUDfEe7ME=;\n\tb=UDEAgqJzUxLvGrL1gacIqOzsriJRKi1/r1CD7k9sml2+TVj0cIBN52tY+9s17hMe04\n\tx4kTv2NaHPQSckLp6RaAxV3HIAUodoosTXcHfnE+AI0tVyJgFMWsQxg0ePtNmxmpxIGp\n\tvYdpumz+IXJ9fo/tWm+dixmcx5EfAyygYSMpnejIeTuGcPhsWzRfGseRHqTRlmkdD+Ln\n\tHZK+Jd6u90Q7578lzw5RXlRb72tWqKgv6Ihc8FdcKkRlpYL0XDa67ZLz9AqP5extRt9V\n\tTqOGdM+On1sbrLKIktlR66rhL8BJhBeJmRKMNQ1273Y/ebEJfgLZX+uh4ug14LLrKhsP\n\t74eg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=7+GCUJI05LqWCHEsqvMhHA6ZZGztXnC5zQBUDfEe7ME=;\n\tb=fB+9/bEYnTcMwGCP4bQQatkqusdF1MlDU9hFLleoHhOxGZBcSkOwF3uMUwKxYTa/H+\n\tVy8Ia1bi7vT76DsjI+EjV3AIIgGL47AsoJqSQGh70wub12FJuphYd0JQeO6q1vmjbuI0\n\tHNhTi4MEICwE2uincCbdJNTYdjf8RTHvPY0pOm//O0mJ/ORnCJsmD3EawtDS23Ug9u++\n\tvr/kKsPUK2PgI2lBikPnVS/py1C8l4yzsy4wtxockXw/HhxEqP4OorXNAfionPhiVNOT\n\tQzFSRGnt/uJ5L9digpDdYGUSJ+N/7wSeGta1fo8qdNHL+PbPFEY231lqm2K0ySARS0RO\n\tEjtQ==","X-Gm-Message-State":"AOAM531p4f5UVXi0MqeK0gA3tdAIG5f+I2hQJl0uSJ+TTrCylPaGqk/Z\n\tE02WRa/tvb/yaQ/6Ysjg6dl3Og==","X-Google-Smtp-Source":"ABdhPJxdJSqH7nPhD2yKKm9xHLpOGsZqB3Z03Bj5ugpkdp1XZdBqKr35i4EhqRy3IuJ7buc20YQuPA==","X-Received":"by 2002:a2e:1556:: with SMTP id 22mr1832210ljv.416.1604621696824;\n\tThu, 05 Nov 2020 16:14:56 -0800 (PST)","Date":"Fri, 6 Nov 2020 01:14:54 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20201106001454.GI2008567@oden.dyn.berto.se>","References":"<20201104074841.21676-1-laurent.pinchart@ideasonboard.com>\n\t<20201104074841.21676-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201104074841.21676-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/3] libcamera: v4l2_videodevice:\n\tZero-initialize planes in V4L2DeviceFormat","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]