[{"id":13205,"web_url":"https://patchwork.libcamera.org/comment/13205/","msgid":"<20201014124207.qevd7kcvnlr2rwk7@oden.dyn.berto.se>","date":"2020-10-14T12:42:07","subject":"Re: [libcamera-devel] [PATCH 09/10] android: metadata: Prevent\n\tvariable aliasing","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nOn 2020-10-13 16:12:40 +0100, Kieran Bingham wrote:\n> The validate_camera_metadata_structure() call uses two instances of a\n> variable named aligned_ptr().\n> \n> Reuse the first instance which is not otherwise re-used for the second\n> scoped usage.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n> *** This is in android metadata library code ***\n> \n> This is only here as a temporary repair. Ideally we shouldn't make changes to this code base.\n> disabling the warning on this library component is likely a better choice.\n> \n>  src/android/metadata/camera_metadata.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> index b86586a7e685..129d1e92ece1 100644\n> --- a/src/android/metadata/camera_metadata.c\n> +++ b/src/android/metadata/camera_metadata.c\n> @@ -433,7 +433,7 @@ int validate_camera_metadata_structure(const camera_metadata_t *metadata,\n>          };\n>  \n>          for (size_t i = 0; i < sizeof(alignments)/sizeof(alignments[0]); ++i) {\n> -            uintptr_t aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,\n> +            aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,\n\nThis is a tricky one. As you say ideally we should fix this upstream and \nbackport it. But being able to enable the build time warning of our own \ncode base would be really nice. As we already inject a SPDX header to \nthis file I see no real harm, with a patch submitted for upstream,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n>                      alignments[i].alignment);\n>  \n>              if ((uintptr_t)metadata + alignmentOffset != aligned_ptr) {\n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D72DBBEEE0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 14 Oct 2020 12:42:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6628760F6E;\n\tWed, 14 Oct 2020 14:42:11 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0186A60E79\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 14 Oct 2020 14:42:09 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id v6so3470355lfa.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 14 Oct 2020 05:42:09 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tz5sm1103325lfj.290.2020.10.14.05.42.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 14 Oct 2020 05:42:08 -0700 (PDT)"],"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=\"FmeVg9e8\"; 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=GWcZglN7XfqmUZL88apqZQB0D7gery6YQfcIZpQ4jzE=;\n\tb=FmeVg9e88TIB7Tfv7vIPM6KAMhJhUHCGTETizTjThQW3U5vs70/xIIj/01+zKeL73n\n\tkfR+zbZXRnMDPZ+E8rtZGUdnYHimGOUeNkn1HtI0u3Y4rbWVDR/J4sNrLZVpH1H0lzk/\n\tMZbVEbFsKeMWiKZaI3PcBdUO6DOgJftf674jPb+8TLnFEDeaRxhLm/b0s9l4gC79QZgA\n\tKN2jQSVU+c6fKppmn2Xj4+2zmclOidaBhLwKucOfvd9SrWxgOuozq0nhECUJflrP3P/E\n\tvdJ9OzLbEt9CfTJYAJGtExcYo0VQLxaiBKGQPKPUIZ5gUJBn+Yx8w4H/89V5eoChsOlp\n\tl6fw==","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=GWcZglN7XfqmUZL88apqZQB0D7gery6YQfcIZpQ4jzE=;\n\tb=lVHR48UTgXA664HsRLHDl5yJ1jxM+lj15DC3v4OLjOBiv/530Kkv6bsiW0p6uFOvBc\n\tZgeDPkIBXm09vaU47eZ5xxeSc85yJ0wmTOq0iFgf9d4i8IMus2Nej85vvSvel5lYRFGl\n\t7HtqsZ8Im6paohmpPg7LwtR2rkZ34g39HKjVdRtWRSLGMl9Jxtfx7SifRq1w52F23qCd\n\tROoOv1PpIRsLUKpkbtY2fBNlYqoHn5ydwJjhkm+WFVkksTPmQFLS2GhYu7IxW/OpytIC\n\tEpJ//IahdknuyLOgbsV6W5qU65X+ydLr6+mtn26WEoHRgQito+eZan2FT3dteuNbX5C5\n\tFpyA==","X-Gm-Message-State":"AOAM531B9xrGYlm6aW4yQHRjpTDrbNzLG/derkDKKu/gWGi3xCjw96QA\n\tUAUdgIZ6fdCHnagL3bKfMjYqTw==","X-Google-Smtp-Source":"ABdhPJwIPlZVVCNq8Bdm8XYZc6lG9Q9FPFuDUe794tOeT8aI6zAtWnJprfveJX0JQZ/3sceSbu8NPw==","X-Received":"by 2002:ac2:4852:: with SMTP id 18mr1259151lfy.341.1602679329407;\n\tWed, 14 Oct 2020 05:42:09 -0700 (PDT)","Date":"Wed, 14 Oct 2020 14:42:07 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201014124207.qevd7kcvnlr2rwk7@oden.dyn.berto.se>","References":"<20201013151241.3557005-1-kieran.bingham@ideasonboard.com>\n\t<20201013151241.3557005-10-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201013151241.3557005-10-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 09/10] android: metadata: Prevent\n\tvariable aliasing","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13214,"web_url":"https://patchwork.libcamera.org/comment/13214/","msgid":"<20201015134224.GE3957@pendragon.ideasonboard.com>","date":"2020-10-15T13:42:24","subject":"Re: [libcamera-devel] [PATCH 09/10] android: metadata: Prevent\n\tvariable aliasing","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Oct 14, 2020 at 02:42:07PM +0200, Niklas Söderlund wrote:\n> On 2020-10-13 16:12:40 +0100, Kieran Bingham wrote:\n> > The validate_camera_metadata_structure() call uses two instances of a\n> > variable named aligned_ptr().\n> > \n> > Reuse the first instance which is not otherwise re-used for the second\n> > scoped usage.\n> > \n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> > *** This is in android metadata library code ***\n> > \n> > This is only here as a temporary repair. Ideally we shouldn't make changes to this code base.\n> > disabling the warning on this library component is likely a better choice.\n> > \n> >  src/android/metadata/camera_metadata.c | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> > index b86586a7e685..129d1e92ece1 100644\n> > --- a/src/android/metadata/camera_metadata.c\n> > +++ b/src/android/metadata/camera_metadata.c\n> > @@ -433,7 +433,7 @@ int validate_camera_metadata_structure(const camera_metadata_t *metadata,\n> >          };\n> >  \n> >          for (size_t i = 0; i < sizeof(alignments)/sizeof(alignments[0]); ++i) {\n> > -            uintptr_t aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,\n> > +            aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,\n> \n> This is a tricky one. As you say ideally we should fix this upstream and \n> backport it. But being able to enable the build time warning of our own \n> code base would be really nice. As we already inject a SPDX header to \n> this file I see no real harm, with a patch submitted for upstream,\n> \n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nAlternatively we could disable the warning for the Android camera\nmetadata library.\n\n> >                      alignments[i].alignment);\n> >  \n> >              if ((uintptr_t)metadata + alignmentOffset != aligned_ptr) {","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 53954BE174\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Oct 2020 13:43:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B080960E8D;\n\tThu, 15 Oct 2020 15:43:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89D54605BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Oct 2020 15:43:11 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 08C84556;\n\tThu, 15 Oct 2020 15:43:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MyxKx5Gf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602769391;\n\tbh=FrcfC70NZPaAF07nmYZ/IOkvJe8UyyZ10lGwJfTiyMU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MyxKx5GfmwR2kkUKv8O1vCvaHk9xiBOwtHSJuyWJEAR7HqTWwvaGB5AFKjbw+4xhI\n\tRajQqED5cJuT85Ugh8NOmi4MbCzRkr4DAod0WIFHiqod6cq8nWt8sg/LIANScM7FAg\n\tdhoPNJFzffdxBc5Q1Zj5GXAaJlFqBtJyeTrSWWwU=","Date":"Thu, 15 Oct 2020 16:42:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20201015134224.GE3957@pendragon.ideasonboard.com>","References":"<20201013151241.3557005-1-kieran.bingham@ideasonboard.com>\n\t<20201013151241.3557005-10-kieran.bingham@ideasonboard.com>\n\t<20201014124207.qevd7kcvnlr2rwk7@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201014124207.qevd7kcvnlr2rwk7@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 09/10] android: metadata: Prevent\n\tvariable aliasing","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]