[{"id":27759,"web_url":"https://patchwork.libcamera.org/comment/27759/","msgid":"<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>","date":"2023-09-12T13:21:57","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi George\n\nOn Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> this definition conflicts with that:\n>\n> :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> [-Werror,-Wmacro-redefined]\n>\n> Rather than adding logic to keep up with their local configuration, it\n> seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n>\n> Signed-off-by: George Burgess IV \n> ---\n> meson.build | 24 +++++++++++++++++++-----\n> 1 file changed, 19 insertions(+), 5 deletions(-)\n>\n> diff --git a/meson.build b/meson.build\n> index 7959b538..2e834263 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> error('clang version is too old, libcamera requires 9.0 or newer')\n> endif\n>\n> - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> - # or higher). This is needed on clang only as gcc enables it by default.\n> + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> + # result in macro redefinition errors if the user already has a setting for\n> + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> if get_option('optimization') != '0'\n> - common_arguments += [\n> - '-D_FORTIFY_SOURCE=2',\n> - ]\n> + has_fortify_define = false\n> + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> + # do the same for c_args.\n> + foreach flag : get_option('cpp_args')\n> + if flag == '-U_FORTIFY_SOURCE'\n> + has_fortify_define = false\n\nDo you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\nsame command line, so that you have to reset has_fortify_define to\nfalse if it was previously been set to true here below ?\n\nOtherwise I might be missing why you have to set 'has_fortify_define =\nfalse' when the variable is already initialized to 'false'...\n\n> + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> + has_fortify_define = true\n> + endif\n> + endforeach\n> + if not has_fortify_define\n> + common_arguments += [\n> + '-D_FORTIFY_SOURCE=2',\n> + ]\n> + endif\n> endif\n>\n> # Use libc++ by default if available instead of libstdc++ when compiling\n> --\n> 2.42.0.283.g2d96d420d3-goog\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 79451BE080\n\tfor ;\n\tTue, 12 Sep 2023 13:22:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2BEA628EC;\n\tTue, 12 Sep 2023 15:22:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E52461DEF\n\tfor ;\n\tTue, 12 Sep 2023 15:22:01 +0200 (CEST)","from ideasonboard.com (mob-5-90-67-213.net.vodafone.it\n\t[5.90.67.213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03A67291;\n\tTue, 12 Sep 2023 15:20:29 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694524922;\n\tbh=UMXHBIX8gZwXQvE7ZtRdc4mUT/lFhR+M+0F4CGmItdc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=RR1hU8SfN772Z89SdIZMemFhnsUYhMkpCxmYEjQ+uuYsdw9Kw1Wr5KE99HLzgMUQi\n\tsVoRkOt1EtgUXsOPBlLCN5cq2X0heGFZsz/uINg6KopodoS502JuvPb6C4Q2MVy8KT\n\tl197u4516ilegFkQxx74kcRKioeXFWQKkCyPtXSitOSLREao1bETbVlJQyDzymhFa0\n\tORm314LJr4DrKEGUYHhJoqyB9bLAKmspnGMNbKaDy6+sXoxkjl7sX4o1uNMfUWsWjq\n\tiIwq4nJsgh+iQq/1kTySaRTRa9GctT3CpeJZ54a7SHg8xSK8+vOxPY8EjRYGn93jv4\n\tOhF3d6eSUyqxQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694524830;\n\tbh=UMXHBIX8gZwXQvE7ZtRdc4mUT/lFhR+M+0F4CGmItdc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AV1Bsx42y9lGeo4AewqmALSJVfdgfff8uK4dymtKXEgdSnRk/bMEJvw23xSSHUCDT\n\ty7po2VnJ8M6Kt11MBq3jEBEYamtgIGDAArVm7KjJwIRPyKqwSYW2T69tU/gKj/ccA5\n\twsNSYBA5fRwFu3NvU3JEc9uZl8ncnE1fD1c0ZfTI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AV1Bsx42\"; dkim-atps=neutral","Date":"Tue, 12 Sep 2023 15:21:57 +0200","To":"George Burgess IV ","Message-ID":"<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>","References":"<20230911230909.1263308-1-gbiv@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230911230909.1263308-1-gbiv@google.com>","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27763,"web_url":"https://patchwork.libcamera.org/comment/27763/","msgid":"","date":"2023-09-12T15:06:20","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":172,"url":"https://patchwork.libcamera.org/api/people/172/","name":"George Burgess","email":"gbiv@google.com"},"content":"Hey,\n\n> Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> same command line, so that you have to reset has_fortify_define to\n> false if it was previously been set to true here below ?\n\nYou're correct - the code as written intends to answer \"will FORTIFY _be\nenabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\nthe code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\nwould work equally well for ChromeOS. :)\n\nGeorge\n\nOn Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n wrote:\n>\n> Hi George\n>\n> On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > this definition conflicts with that:\n> >\n> > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > [-Werror,-Wmacro-redefined]\n> >\n> > Rather than adding logic to keep up with their local configuration, it\n> > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> >\n> > Signed-off-by: George Burgess IV \n> > ---\n> > meson.build | 24 +++++++++++++++++++-----\n> > 1 file changed, 19 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/meson.build b/meson.build\n> > index 7959b538..2e834263 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > error('clang version is too old, libcamera requires 9.0 or newer')\n> > endif\n> >\n> > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > - # or higher). This is needed on clang only as gcc enables it by default.\n> > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > + # result in macro redefinition errors if the user already has a setting for\n> > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > if get_option('optimization') != '0'\n> > - common_arguments += [\n> > - '-D_FORTIFY_SOURCE=2',\n> > - ]\n> > + has_fortify_define = false\n> > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > + # do the same for c_args.\n> > + foreach flag : get_option('cpp_args')\n> > + if flag == '-U_FORTIFY_SOURCE'\n> > + has_fortify_define = false\n>\n> Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> same command line, so that you have to reset has_fortify_define to\n> false if it was previously been set to true here below ?\n>\n> Otherwise I might be missing why you have to set 'has_fortify_define =\n> false' when the variable is already initialized to 'false'...\n>\n> > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > + has_fortify_define = true\n> > + endif\n> > + endforeach\n> > + if not has_fortify_define\n> > + common_arguments += [\n> > + '-D_FORTIFY_SOURCE=2',\n> > + ]\n> > + endif\n> > endif\n> >\n> > # Use libc++ by default if available instead of libstdc++ when compiling\n> > --\n> > 2.42.0.283.g2d96d420d3-goog\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 73DC1BD160\n\tfor ;\n\tTue, 12 Sep 2023 15:07:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F11B61DF5;\n\tTue, 12 Sep 2023 17:07:00 +0200 (CEST)","from mail-oa1-x32.google.com (mail-oa1-x32.google.com\n\t[IPv6:2001:4860:4864:20::32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6B5B61DEF\n\tfor ;\n\tTue, 12 Sep 2023 17:06:57 +0200 (CEST)","by mail-oa1-x32.google.com with SMTP id\n\t586e51a60fabf-1d598ba1b74so2016536fac.0\n\tfor ;\n\tTue, 12 Sep 2023 08:06:57 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694531220;\n\tbh=s5mzGPC/OqDceu/RX7cpU79eDyKkd4R4xde5YUVrvxM=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ljefLauq2CnzM1FoLr3thfgE6rUopRWNU1xAjldx9fKX6+EKZNwuGMLrGAIK7iPRd\n\tkwjhq47KcJ8AjlFtNQBQ91Q7BG07Dp2h4T+A7eOBP2eb5Ot2rSHWByuciOP4kKTD0h\n\tE+iHkmKM+oiutoWrZ0KzFjkx1B98O2D8c+Jwx6feiuiVyQKmqlZQvr9F4N3Ff/4DLW\n\tRXczo8cFAl5vryBtRBiV54Gje3Tpjyue9l+diZOFYHLbqB8etVRFPBPdff8jileWt4\n\tWBloyWaONZ8SFYpQJ7RKQCZJn341eIL7egO9DvckTYf0iwXuLBAxr+4lgRjCHrJFK0\n\tkfc3tVSZFY/UA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1694531216; x=1695136016;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=Ss+1iQG3iC5mTStBVLQIpl9bQC0C1HC/5nUJhf61Ax4=;\n\tb=usn5x1z06FxQSLfA4mVv1w7L0FDyOV9PBT0sRuzXiFHPYxbd7HkN3/yGNezAvgY4qm\n\tP5P55k7kPYVD8jqxkSr1KBqXi4aiOWF7fsc3M41nC6EoKwUFi3pkMiIckpGtOJ+bl9Ph\n\tgJu5Z7n9pJANUUyUc9dAqLrZoZcZyApJzik3QRD0XrAXepI46QDytQgRrX5r0qi4TryF\n\tdmGBGFMikgy7WjFVsKmjum/cwljLCp8mjcxAASf1jKxCQ40ZhNgs4kzenW8YX+TSBVrm\n\teedtrgOVE5DKTeYSqCZ0FvfVlNEtsIEvv0UO5FgFIEyKfN8hBABVrUQgAbs8cNUwgijo\n\tJvXQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=google.com\n\theader.i=@google.com header.b=\"usn5x1z0\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694531216; x=1695136016;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=Ss+1iQG3iC5mTStBVLQIpl9bQC0C1HC/5nUJhf61Ax4=;\n\tb=ntXymanq2+hdyHvl6yhEabDKslBjiSbTUhf4l9Rrn8UxaBURq3TX3CzXOhUjutlkWy\n\tN9vqaGtdC9ETAzL/3qadoerkRh08n3ob/jCp11RlNjCpEbeN7ZQJtmivDiCT1y/j2iIo\n\tNljCFIjUafVTf6FXRP0zPW+aqHQN6N0+zA104W4s7FHL6MmHBpBumqT21UmW9gUDpUNj\n\to0dgjPwQmziktMmmuhOqDOrC6jwOdxtQQsVB0AgHr28HEQqC8lwFGRiA9As4UntodUnC\n\tdQ87aPoOO38yTRA77Qlnl+V9FzzKtpyjslKWS6Dfom5F6JSEuenRdJE1gAnPTX4ULVyV\n\tRm8w==","X-Gm-Message-State":"AOJu0YzRG89N1xT68wuKsakVX00vgH1TIhMS/maSS0r3CW0vH2GUDlcD\n\tl5JDHMYQ1t3LWy6A4EEiWf6uoJrjTfvyK7oeC1Wyzw==","X-Google-Smtp-Source":"AGHT+IGQiNc9rsEsb1euKOsFDqVGjpfQ7GSXfI6J4tIZuurwMEK92VpAp6qzLiglaXhbopRyzDWO4OXssJ3c4UGINAs=","X-Received":"by 2002:a05:6870:4250:b0:17f:7388:4c69 with SMTP id\n\tv16-20020a056870425000b0017f73884c69mr16871410oac.30.1694531216227;\n\tTue, 12 Sep 2023 08:06:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>","In-Reply-To":"<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>","Date":"Tue, 12 Sep 2023 09:06:20 -0600","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"George Burgess via libcamera-devel\n\t","Reply-To":"George Burgess ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27767,"web_url":"https://patchwork.libcamera.org/comment/27767/","msgid":"","date":"2023-09-14T07:53:15","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi George\n\nOn Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> Hey,\n>\n> > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > same command line, so that you have to reset has_fortify_define to\n> > false if it was previously been set to true here below ?\n>\n> You're correct - the code as written intends to answer \"will FORTIFY _be\n> enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> would work equally well for ChromeOS. :)\n\nPlease do not mangle the email content and try to reply where the\ncomment was made in the patch, otherwise it's hard to find context.\n\nI'll copy your reply below.\n\n>\n> George\n>\n> On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n> wrote:\n> >\n> > Hi George\n> >\n> > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > this definition conflicts with that:\n> > >\n> > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > [-Werror,-Wmacro-redefined]\n> > >\n> > > Rather than adding logic to keep up with their local configuration, it\n> > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > >\n> > > Signed-off-by: George Burgess IV \n> > > ---\n> > > meson.build | 24 +++++++++++++++++++-----\n> > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/meson.build b/meson.build\n> > > index 7959b538..2e834263 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > endif\n> > >\n> > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > + # result in macro redefinition errors if the user already has a setting for\n> > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > if get_option('optimization') != '0'\n> > > - common_arguments += [\n> > > - '-D_FORTIFY_SOURCE=2',\n> > > - ]\n> > > + has_fortify_define = false\n> > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > + # do the same for c_args.\n> > > + foreach flag : get_option('cpp_args')\n> > > + if flag == '-U_FORTIFY_SOURCE'\n> > > + has_fortify_define = false\n> >\n> > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > same command line, so that you have to reset has_fortify_define to\n> > false if it was previously been set to true here below ?\n> >\n> > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > false' when the variable is already initialized to 'false'...\n> >\n>\n> You're correct - the code as written intends to answer \"will FORTIFY _be\n> enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> would work equally well for ChromeOS. :)\n\nAs I read this the code is written to protect against the case where\n-U_FORTIFY_SOURCE is specified in the same command line -after-\n-D_FORTIFY_SOURCE, isn't it ?\n\nMy comment was about the fact `has_fortify_define` is already\ninitialized to false, so there is not need for\n\n if flag == '-U_FORTIFY_SOURCE'\n has_fortify_define = false\n\nUnless you don't expect the above described case.\n\nAs command lines are usually generated by the build system and\nmultiple options to enable/disable a feature might be concatenated in\nthe same line, I'm not opposed to this, but I was wondering if this\nwas by design or not.\n\n\n> > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > + has_fortify_define = true\n> > > + endif\n> > > + endforeach\n> > > + if not has_fortify_define\n> > > + common_arguments += [\n> > > + '-D_FORTIFY_SOURCE=2',\n> > > + ]\n> > > + endif\n> > > endif\n> > >\n> > > # Use libc++ by default if available instead of libstdc++ when compiling\n> > > --\n> > > 2.42.0.283.g2d96d420d3-goog\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 D9585BE080\n\tfor ;\n\tThu, 14 Sep 2023 07:53:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 056D0628F5;\n\tThu, 14 Sep 2023 09:53:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60263628DA\n\tfor ;\n\tThu, 14 Sep 2023 09:53:19 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A086EDE5;\n\tThu, 14 Sep 2023 09:51:46 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694678001;\n\tbh=a7ekkfkRE4ARTvwGOka8zGWwaTsBuN/syUykUn+e6ZI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=v3fgQwNut/jel+kiRyGxfrdSKH3P9H07PSM/MtVI0cf/lINRshhnT32XPdxhcQ6gN\n\t90xY/YNbwNTFODfoORcPM4Eqr2Ao7glUtnGmeUZNgBna99p2Bj3tRQJ9HzcWyCzruU\n\thu5tV1ze3OqMjFugoH5zPfUA+Ix7+EcqkFuO35qewnPFnQnkioYyzhM8/jAuhkU9TP\n\tt0nO3aYtQ3HxInkN1QyODo683RGOsbXhyLFe02Rr5as7msuDzmToL1llN3jSJBGrsU\n\t0cmZTYtn4B55ZDFhs3LWVFf8Ycx91CfuV0jy5Y++TNSaxytYsVtKJ9K1Ww37b2/DUJ\n\tND8XqOyApfaFA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694677906;\n\tbh=a7ekkfkRE4ARTvwGOka8zGWwaTsBuN/syUykUn+e6ZI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=S9gtVH+vg6Qh3KReEm/3e5cTSObtCm+01CdL/yOz9+z1To+6wujRkuLG2naYJBkbF\n\tzKfz0vEu2FokEDq6YSGaA2LcuLmQBZn+f1cMvCIPYN/rGqcPq8U8Aug4u9iDRPKhM2\n\t5rJ4hOKFX4n3osXIH4wqeko5Lfow7T7TFr0nlP8U="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"S9gtVH+v\"; dkim-atps=neutral","Date":"Thu, 14 Sep 2023 09:53:15 +0200","To":"George Burgess ","Message-ID":"","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27772,"web_url":"https://patchwork.libcamera.org/comment/27772/","msgid":"","date":"2023-09-14T14:48:49","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":172,"url":"https://patchwork.libcamera.org/api/people/172/","name":"George Burgess","email":"gbiv@google.com"},"content":"Hey Jacopo,\n\nOn Thu, Sep 14, 2023 at 1:53 AM Jacopo Mondi\n wrote:\n>\n> Hi George\n>\n> On Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> > Hey,\n> >\n> > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > same command line, so that you have to reset has_fortify_define to\n> > > false if it was previously been set to true here below ?\n> >\n> > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > would work equally well for ChromeOS. :)\n>\n> Please do not mangle the email content and try to reply where the\n> comment was made in the patch, otherwise it's hard to find context.\n\nAh, is replying inline like this the preferred style? I'll try to do that going\nforward then - thanks for the tip.\n\n>\n> I'll copy your reply below.\n>\n> >\n> > George\n> >\n> > On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi George\n> > >\n> > > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > > this definition conflicts with that:\n> > > >\n> > > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > > [-Werror,-Wmacro-redefined]\n> > > >\n> > > > Rather than adding logic to keep up with their local configuration, it\n> > > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > > >\n> > > > Signed-off-by: George Burgess IV \n> > > > ---\n> > > > meson.build | 24 +++++++++++++++++++-----\n> > > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > > >\n> > > > diff --git a/meson.build b/meson.build\n> > > > index 7959b538..2e834263 100644\n> > > > --- a/meson.build\n> > > > +++ b/meson.build\n> > > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > > endif\n> > > >\n> > > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > > + # result in macro redefinition errors if the user already has a setting for\n> > > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > > if get_option('optimization') != '0'\n> > > > - common_arguments += [\n> > > > - '-D_FORTIFY_SOURCE=2',\n> > > > - ]\n> > > > + has_fortify_define = false\n> > > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > > + # do the same for c_args.\n> > > > + foreach flag : get_option('cpp_args')\n> > > > + if flag == '-U_FORTIFY_SOURCE'\n> > > > + has_fortify_define = false\n> > >\n> > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > same command line, so that you have to reset has_fortify_define to\n> > > false if it was previously been set to true here below ?\n> > >\n> > > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > > false' when the variable is already initialized to 'false'...\n> > >\n> >\n> > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > would work equally well for ChromeOS. :)\n>\n> As I read this the code is written to protect against the case where\n> -U_FORTIFY_SOURCE is specified in the same command line -after-\n> -D_FORTIFY_SOURCE, isn't it ?\n>\n> My comment was about the fact `has_fortify_define` is already\n> initialized to false, so there is not need for\n>\n> if flag == '-U_FORTIFY_SOURCE'\n> has_fortify_define = false\n>\n> Unless you don't expect the above described case.\n>\n> As command lines are usually generated by the build system and\n> multiple options to enable/disable a feature might be concatenated in\n> the same line, I'm not opposed to this, but I was wondering if this\n> was by design or not.\n\nYes, this is by design, and I wrote it this way for the exact reason you\nmention: build flags can be gathered from many different places, and later ones\ntake precedence over earlier ones. This logic tries to recognize that and handle\nit appropriately.\n\n>\n>\n> > > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > > + has_fortify_define = true\n> > > > + endif\n> > > > + endforeach\n> > > > + if not has_fortify_define\n> > > > + common_arguments += [\n> > > > + '-D_FORTIFY_SOURCE=2',\n> > > > + ]\n> > > > + endif\n> > > > endif\n> > > >\n> > > > # Use libc++ by default if available instead of libstdc++ when compiling\n> > > > --\n> > > > 2.42.0.283.g2d96d420d3-goog\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 D8B3CBD160\n\tfor ;\n\tThu, 14 Sep 2023 14:49:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B2B1628ED;\n\tThu, 14 Sep 2023 16:49:29 +0200 (CEST)","from mail-vk1-xa2c.google.com (mail-vk1-xa2c.google.com\n\t[IPv6:2607:f8b0:4864:20::a2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD249628DA\n\tfor ;\n\tThu, 14 Sep 2023 16:49:26 +0200 (CEST)","by mail-vk1-xa2c.google.com with SMTP id\n\t71dfb90a1353d-48faba23f51so512322e0c.1\n\tfor ;\n\tThu, 14 Sep 2023 07:49:26 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694702969;\n\tbh=zX0bbrgkwzlYVAaGUBe4M92SfOBTbziATNwnwVoDdBI=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=e6ZA7ATAqWXbGq542xOzK3589t1ljwnEwSdgwP0KHQNP9bhpf9tM0UGv+rStaTDTQ\n\tx137DxK9SUXKVPKR1Q1am+OgvUJyHXBpBmHs2LUeFcPQ+NeBXO5/tmsNIxDF9LUvGp\n\tTrUoDwJiAe3hPdXsbxD4NegFts6Dz1q27r4dMh4X9Itia7xT+s1Km4rfZCMag6uGDl\n\tKJZcJez70W6MQ6zT+wJ6Nb8feTyyUz1N63JrIJDD5Vt0sUFM1H1cnHNFQHb0iwGHvf\n\t9WcOatQ72vbN4Mliu5o2RPdKrlAOGSEcfMwtKLaclLWmlG3WtO/dh7AJIOcRTu9lwx\n\t9au1PbBS9Q5Wg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1694702965; x=1695307765;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=pWVYvpkHgAAJtA1wvGf1xnSaUjWTJ9bGEp1b/vtcGOA=;\n\tb=lSlVaiJJLRIGFAHWAtFJG13v+p3cokiyWx16m4CJc0dvhaP8UERTm+3P8dz7ri84U7\n\tcJkmr8C9tCCuJSi2YCoyQdsk2yIoq5rr+aruXlTJc8YLRVhmSfcLacVGz6QLtMAQDse6\n\tNNNsgM4n7afNLaOXgIisGtRzr1w8vEES2vingSKRWSeqmmLBEBnSRImFK56T45hoFQBA\n\tFb8oN6ydrez2Lzi3rP7vK+IGpDJHuqzP74UgoBciKo5hrAyNMXo+2vELl6nW3zjZj3uA\n\tGzvVliWimn0gVDRzfF6hkhddJM3wRiUF1+aYXxvcjnGL4uwAHfMg4XuoT5RJN9bs6Ggu\n\tSsUg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=google.com\n\theader.i=@google.com header.b=\"lSlVaiJJ\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694702965; x=1695307765;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=pWVYvpkHgAAJtA1wvGf1xnSaUjWTJ9bGEp1b/vtcGOA=;\n\tb=Sb0P5Jd043E+Xx2XjSJ7GBddwVO6Z8vDh5JRXAnqZv4t6gz+kSxAkWc9Sh7rgq3ZHV\n\ttA9RtsQiGMUFpla7UdKmQ+ab+kinDafyxi+tFfur8DZMhINFvV/4x7XmqxnE9PkKZkmJ\n\toCxhk06WwafVcVxgvjUqgSW2rvgO2swqrDNLZ5x0h7stRVNIcK3XJv+g9hwxL/3h4e6T\n\ttQR26E7jDM/gpw0uGKNP8VecENMQ1t5mht3SZ8pc2Q1X0Rb083QQsSSwgRAhLpud4Ann\n\tsmwczh52SBcP/EKTUHOLlsp8fryIN+bT5U1P2mNGbrDNpRDVzXl/pRZ/oUpStmo86i1G\n\twu1g==","X-Gm-Message-State":"AOJu0YyPWb/x6CBaoO8kwkG/atG0Bv9qucyDcff5FwVin0dfxFnrGNPB\n\t7qPJWuHwa2DsuZFhVCyHXX7+/v/7YohaxKFgJm446A==","X-Google-Smtp-Source":"AGHT+IE/jVkIvUhskRsO2rjgMlN/uw/RsYB+AGkYe2unQZrFmUEP8AB0ADG2vAdlqFemGkmrj+Q4yHCnhrEg7tTrdpQ=","X-Received":"by 2002:a1f:4b02:0:b0:493:7c76:bbac with SMTP id\n\ty2-20020a1f4b02000000b004937c76bbacmr5371908vka.2.1694702965460;\n\tThu, 14 Sep 2023 07:49:25 -0700 (PDT)","MIME-Version":"1.0","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t\n\t","In-Reply-To":"","Date":"Thu, 14 Sep 2023 08:48:49 -0600","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"George Burgess via libcamera-devel\n\t","Reply-To":"George Burgess ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27773,"web_url":"https://patchwork.libcamera.org/comment/27773/","msgid":"","date":"2023-09-14T15:32:11","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi George\n\nOn Thu, Sep 14, 2023 at 08:48:49AM -0600, George Burgess via libcamera-devel wrote:\n> Hey Jacopo,\n>\n> On Thu, Sep 14, 2023 at 1:53 AM Jacopo Mondi\n> wrote:\n> >\n> > Hi George\n> >\n> > On Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> > > Hey,\n> > >\n> > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > same command line, so that you have to reset has_fortify_define to\n> > > > false if it was previously been set to true here below ?\n> > >\n> > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > would work equally well for ChromeOS. :)\n> >\n> > Please do not mangle the email content and try to reply where the\n> > comment was made in the patch, otherwise it's hard to find context.\n>\n> Ah, is replying inline like this the preferred style? I'll try to do that going\n> forward then - thanks for the tip.\n>\n> >\n> > I'll copy your reply below.\n> >\n> > >\n> > > George\n> > >\n> > > On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n> > > wrote:\n> > > >\n> > > > Hi George\n> > > >\n> > > > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > > > this definition conflicts with that:\n> > > > >\n> > > > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > > > [-Werror,-Wmacro-redefined]\n> > > > >\n> > > > > Rather than adding logic to keep up with their local configuration, it\n> > > > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > > > >\n> > > > > Signed-off-by: George Burgess IV \n> > > > > ---\n> > > > > meson.build | 24 +++++++++++++++++++-----\n> > > > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > > > >\n> > > > > diff --git a/meson.build b/meson.build\n> > > > > index 7959b538..2e834263 100644\n> > > > > --- a/meson.build\n> > > > > +++ b/meson.build\n> > > > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > > > endif\n> > > > >\n> > > > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > > > + # result in macro redefinition errors if the user already has a setting for\n> > > > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > > > if get_option('optimization') != '0'\n> > > > > - common_arguments += [\n> > > > > - '-D_FORTIFY_SOURCE=2',\n> > > > > - ]\n> > > > > + has_fortify_define = false\n> > > > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > > > + # do the same for c_args.\n> > > > > + foreach flag : get_option('cpp_args')\n> > > > > + if flag == '-U_FORTIFY_SOURCE'\n> > > > > + has_fortify_define = false\n> > > >\n> > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > same command line, so that you have to reset has_fortify_define to\n> > > > false if it was previously been set to true here below ?\n> > > >\n> > > > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > > > false' when the variable is already initialized to 'false'...\n> > > >\n> > >\n> > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > would work equally well for ChromeOS. :)\n> >\n> > As I read this the code is written to protect against the case where\n> > -U_FORTIFY_SOURCE is specified in the same command line -after-\n> > -D_FORTIFY_SOURCE, isn't it ?\n> >\n> > My comment was about the fact `has_fortify_define` is already\n> > initialized to false, so there is not need for\n> >\n> > if flag == '-U_FORTIFY_SOURCE'\n> > has_fortify_define = false\n> >\n> > Unless you don't expect the above described case.\n> >\n> > As command lines are usually generated by the build system and\n> > multiple options to enable/disable a feature might be concatenated in\n> > the same line, I'm not opposed to this, but I was wondering if this\n> > was by design or not.\n>\n> Yes, this is by design, and I wrote it this way for the exact reason you\n> mention: build flags can be gathered from many different places, and later ones\n> take precedence over earlier ones. This logic tries to recognize that and handle\n> it appropriately.\n>\n\nOk, just wanted to make sure this was by design, and I think\nprotecting against conflicting parameters makes sense!\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n> >\n> >\n> > > > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > > > + has_fortify_define = true\n> > > > > + endif\n> > > > > + endforeach\n> > > > > + if not has_fortify_define\n> > > > > + common_arguments += [\n> > > > > + '-D_FORTIFY_SOURCE=2',\n> > > > > + ]\n> > > > > + endif\n> > > > > endif\n> > > > >\n> > > > > # Use libc++ by default if available instead of libstdc++ when compiling\n> > > > > --\n> > > > > 2.42.0.283.g2d96d420d3-goog\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 6FDE9BE080\n\tfor ;\n\tThu, 14 Sep 2023 15:32:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B23B3628FF;\n\tThu, 14 Sep 2023 17:32:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 13B79628DA\n\tfor ;\n\tThu, 14 Sep 2023 17:32:31 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C51B10FE;\n\tThu, 14 Sep 2023 17:30:57 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694705552;\n\tbh=KNdPN2U3pNclFbK3vzclNe4vWMjOYQuHIjmPGtJzt24=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=kINK2z3jd0HqwMmgwKY03JehkwQNFtlV9FW5k0frFpw0kfKZIajikWBS6PLdUSfAR\n\t8Z8WJUyQWwaBudJZuKWcWXr0FGFxy4KTv30W71hbR2u9JNrArhVnkZngIZeUs2xzQ0\n\tcpbSCWoTCOjuQ5L3o6/r1HfEA++VuGnfajUGhTENYoP1hUmv3/5g0mIMKo/FXz7yzs\n\tZx8IdMplftuNRYplUN3prchH3H7PfGROyWeqel1ULJmLtfRSDvHzke0sH1GcF5CLUI\n\tzdT5GHWx6cyjiX7ZjBS1MrJyBnuV2VQ+7FtNqQBAaRZ++CUSA7x+R2r1mPSBq0ZJkk\n\ti/7u4JPwjcrZA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694705458;\n\tbh=KNdPN2U3pNclFbK3vzclNe4vWMjOYQuHIjmPGtJzt24=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JTJig1qkr+tw+0aD9VsB52FfCGenaFLpOmyLb7GENWNjriKRJ5Z7w9CqRrM1qKUFd\n\ti0VncEm+68ID6pNslmesIEcHUPXNkFVoAFecAvw7IHydLFbe/oe8WJyFGdgt6lvIct\n\tB/JUKH+K/D89K1WDa0HNpeDfFgfHkZ+ndKzfHJU8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JTJig1qk\"; dkim-atps=neutral","Date":"Thu, 14 Sep 2023 17:32:11 +0200","To":"George Burgess ","Message-ID":"","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27814,"web_url":"https://patchwork.libcamera.org/comment/27814/","msgid":"","date":"2023-09-18T15:56:46","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":172,"url":"https://patchwork.libcamera.org/api/people/172/","name":"George Burgess","email":"gbiv@google.com"},"content":"On Thu, Sep 14, 2023 at 9:32 AM Jacopo Mondi\n wrote:\n>\n> Hi George\n>\n> On Thu, Sep 14, 2023 at 08:48:49AM -0600, George Burgess via libcamera-devel wrote:\n> > Hey Jacopo,\n> >\n> > On Thu, Sep 14, 2023 at 1:53 AM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi George\n> > >\n> > > On Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> > > > Hey,\n> > > >\n> > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > false if it was previously been set to true here below ?\n> > > >\n> > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > would work equally well for ChromeOS. :)\n> > >\n> > > Please do not mangle the email content and try to reply where the\n> > > comment was made in the patch, otherwise it's hard to find context.\n> >\n> > Ah, is replying inline like this the preferred style? I'll try to do that going\n> > forward then - thanks for the tip.\n> >\n> > >\n> > > I'll copy your reply below.\n> > >\n> > > >\n> > > > George\n> > > >\n> > > > On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n> > > > wrote:\n> > > > >\n> > > > > Hi George\n> > > > >\n> > > > > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > > > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > > > > this definition conflicts with that:\n> > > > > >\n> > > > > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > > > > [-Werror,-Wmacro-redefined]\n> > > > > >\n> > > > > > Rather than adding logic to keep up with their local configuration, it\n> > > > > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > > > > >\n> > > > > > Signed-off-by: George Burgess IV \n> > > > > > ---\n> > > > > > meson.build | 24 +++++++++++++++++++-----\n> > > > > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > > > > >\n> > > > > > diff --git a/meson.build b/meson.build\n> > > > > > index 7959b538..2e834263 100644\n> > > > > > --- a/meson.build\n> > > > > > +++ b/meson.build\n> > > > > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > > > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > > > > endif\n> > > > > >\n> > > > > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > > > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > > > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > > > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > > > > + # result in macro redefinition errors if the user already has a setting for\n> > > > > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > > > > if get_option('optimization') != '0'\n> > > > > > - common_arguments += [\n> > > > > > - '-D_FORTIFY_SOURCE=2',\n> > > > > > - ]\n> > > > > > + has_fortify_define = false\n> > > > > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > > > > + # do the same for c_args.\n> > > > > > + foreach flag : get_option('cpp_args')\n> > > > > > + if flag == '-U_FORTIFY_SOURCE'\n> > > > > > + has_fortify_define = false\n> > > > >\n> > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > false if it was previously been set to true here below ?\n> > > > >\n> > > > > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > > > > false' when the variable is already initialized to 'false'...\n> > > > >\n> > > >\n> > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > would work equally well for ChromeOS. :)\n> > >\n> > > As I read this the code is written to protect against the case where\n> > > -U_FORTIFY_SOURCE is specified in the same command line -after-\n> > > -D_FORTIFY_SOURCE, isn't it ?\n> > >\n> > > My comment was about the fact `has_fortify_define` is already\n> > > initialized to false, so there is not need for\n> > >\n> > > if flag == '-U_FORTIFY_SOURCE'\n> > > has_fortify_define = false\n> > >\n> > > Unless you don't expect the above described case.\n> > >\n> > > As command lines are usually generated by the build system and\n> > > multiple options to enable/disable a feature might be concatenated in\n> > > the same line, I'm not opposed to this, but I was wondering if this\n> > > was by design or not.\n> >\n> > Yes, this is by design, and I wrote it this way for the exact reason you\n> > mention: build flags can be gathered from many different places, and later ones\n> > take precedence over earlier ones. This logic tries to recognize that and handle\n> > it appropriately.\n> >\n>\n> Ok, just wanted to make sure this was by design, and I think\n> protecting against conflicting parameters makes sense!\n>\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n\nThanks again for your help & review! Sorry for my ignorance of kernelish\npatching procedures, but now that this has been reviewed, is there a process I\nshould follow to get it landed? I don't see my change in the history\nof https://git.libcamera.org/libcamera/libcamera.git/ yet (HEAD == 90e0fea6c).\n\n\n> > >\n> > >\n> > > > > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > > > > + has_fortify_define = true\n> > > > > > + endif\n> > > > > > + endforeach\n> > > > > > + if not has_fortify_define\n> > > > > > + common_arguments += [\n> > > > > > + '-D_FORTIFY_SOURCE=2',\n> > > > > > + ]\n> > > > > > + endif\n> > > > > > endif\n> > > > > >\n> > > > > > # Use libc++ by default if available instead of libstdc++ when compiling\n> > > > > > --\n> > > > > > 2.42.0.283.g2d96d420d3-goog\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 A1834C326B\n\tfor ;\n\tMon, 18 Sep 2023 15:57:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B9AD6293D;\n\tMon, 18 Sep 2023 17:57:26 +0200 (CEST)","from mail-vs1-xe2d.google.com (mail-vs1-xe2d.google.com\n\t[IPv6:2607:f8b0:4864:20::e2d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9824A61DE9\n\tfor ;\n\tMon, 18 Sep 2023 17:57:24 +0200 (CEST)","by mail-vs1-xe2d.google.com with SMTP id\n\tada2fe7eead31-4526ae5b0b3so619458137.2\n\tfor ;\n\tMon, 18 Sep 2023 08:57:24 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695052646;\n\tbh=yQmyMsMOvKidUMakKiWQsMzrz3HQ0UwnoV98RaE4Y1s=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=r2AlSrFehiJg971Olt88QSoRHBPBv5kbrydJNOLclziGWTZ4+myQZfDrJ/DXnW1r+\n\tlIP1SKCdOlZe43gmnI6MgnLdOsCycC0lkDpWKBIoToNLi7ICNyyLqgZknGAFiV8jcd\n\tSq7M5mL6uB/pgvwYwEiRdX+EvRYcw8TB89xzAi1FZYqgEesceGU4Vwk21fNHvAlw0R\n\tCvU01VJgt9Ab80FZ1SKeGgM+IwKzWjx0MbEriT2mG9T1l99xarth7ZcU6+OdFkOgcv\n\t+jZ7t63n2D2316Zh5N6KK5sVqrs4tyIS7ycvVcJCxS3fAcn3UTgEVR31ZiAfFjAgPe\n\tktD7NE+CDU/xA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1695052643; x=1695657443;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=pKUkj8HXVzea96AZG7CW7xvJM6c7AR/ZrMw3igpPUd8=;\n\tb=RecAKkiIEuaL/e6SDdXEu71mtXMkIcEhBXWfYDdT/qDaUmvsQkOM6Df63FYxc+4CCh\n\twaTDRLptmv9Gg8+XPASKDHtNiQv7Wr8zaQnKvy+sN7i1OClr52yne+LsipGe4xpQvKii\n\tzwOcc0BujgGa/OmFnBg6z5LtlzYNSQ8ib8OPddB3qRXd15DyZ2nzVP9Xl+35JcZ5Wff1\n\tj4kTfuJYEl3vj615iF8/Uw7PwE/Lbo7hR+T5J3694ZsYc1FWaeAIH10FckTuWloDuapS\n\tYqwmBOThfmXLIlKFNFd8hqCloSC/yGRw6JrMC2hl6/n4O8vfuGyuSXprBaSyeASheKjW\n\tvYNQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=google.com\n\theader.i=@google.com header.b=\"RecAKkiI\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695052643; x=1695657443;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=pKUkj8HXVzea96AZG7CW7xvJM6c7AR/ZrMw3igpPUd8=;\n\tb=WzrkD+BlEvd/5B9nIsZJ76h7ugqsbZi/eb7ghvvnWOdXfucLwuNKj0qoiEp6jAbj+M\n\tZrmMWrfH7KL9AyHL7ejM2JgtYUnw65f5XywmjD8Th9ZgSaUDtO0gZ1f8JCWo6lEvypdW\n\tbdjmUCaE0CXYszZJyBPm3vWuuGjrxBR3Ci7Az1ipjsjyYKy8aej8aRHGW6wGSmYfPSSg\n\tvyn0SwGa1GGDhCmU7GPhQ0VpLUTwujFQxWOqX8CNJ9ouNyskaeXTvuVyfES3su5fHcnS\n\t2qS4mKbPNWGFMEN437u5FOgs439onCt+MBpU/fosCDNux6HUN2NwlrNtSES0uzzj3hUh\n\tOXAw==","X-Gm-Message-State":"AOJu0YxENZ13qcjWp4F2R4uZaLnR8bk2Tb7rDK7dlp9nG9SxkDvQrBL1\n\t8uhvJAwNKvtfF3EoVpYh7qkVzrPGS+SEa+06E9FIiA==","X-Google-Smtp-Source":"AGHT+IHtI6t230g5sEoEcLHs8WCVjOsN0ouSJ5OLWvTn4ugHrqMpDoRMUUhNSuBTieixVQMryawhAZpI46G9RDehD6o=","X-Received":"by 2002:a67:e30e:0:b0:44d:5298:5bfa with SMTP id\n\tj14-20020a67e30e000000b0044d52985bfamr8200103vsf.2.1695052642262;\n\tMon, 18 Sep 2023 08:57:22 -0700 (PDT)","MIME-Version":"1.0","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Mon, 18 Sep 2023 09:56:46 -0600","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"George Burgess via libcamera-devel\n\t","Reply-To":"George Burgess ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27815,"web_url":"https://patchwork.libcamera.org/comment/27815/","msgid":"","date":"2023-09-19T07:08:29","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi George\n\nOn Mon, Sep 18, 2023 at 09:56:46AM -0600, George Burgess wrote:\n> On Thu, Sep 14, 2023 at 9:32 AM Jacopo Mondi\n> wrote:\n> >\n> > Hi George\n> >\n> > On Thu, Sep 14, 2023 at 08:48:49AM -0600, George Burgess via libcamera-devel wrote:\n> > > Hey Jacopo,\n> > >\n> > > On Thu, Sep 14, 2023 at 1:53 AM Jacopo Mondi\n> > > wrote:\n> > > >\n> > > > Hi George\n> > > >\n> > > > On Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> > > > > Hey,\n> > > > >\n> > > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > > false if it was previously been set to true here below ?\n> > > > >\n> > > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > > would work equally well for ChromeOS. :)\n> > > >\n> > > > Please do not mangle the email content and try to reply where the\n> > > > comment was made in the patch, otherwise it's hard to find context.\n> > >\n> > > Ah, is replying inline like this the preferred style? I'll try to do that going\n> > > forward then - thanks for the tip.\n> > >\n> > > >\n> > > > I'll copy your reply below.\n> > > >\n> > > > >\n> > > > > George\n> > > > >\n> > > > > On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi\n> > > > > wrote:\n> > > > > >\n> > > > > > Hi George\n> > > > > >\n> > > > > > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > > > > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > > > > > this definition conflicts with that:\n> > > > > > >\n> > > > > > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > > > > > [-Werror,-Wmacro-redefined]\n> > > > > > >\n> > > > > > > Rather than adding logic to keep up with their local configuration, it\n> > > > > > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > > > > > >\n> > > > > > > Signed-off-by: George Burgess IV \n> > > > > > > ---\n> > > > > > > meson.build | 24 +++++++++++++++++++-----\n> > > > > > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > > > > > >\n> > > > > > > diff --git a/meson.build b/meson.build\n> > > > > > > index 7959b538..2e834263 100644\n> > > > > > > --- a/meson.build\n> > > > > > > +++ b/meson.build\n> > > > > > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > > > > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > > > > > endif\n> > > > > > >\n> > > > > > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > > > > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > > > > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > > > > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > > > > > + # result in macro redefinition errors if the user already has a setting for\n> > > > > > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > > > > > if get_option('optimization') != '0'\n> > > > > > > - common_arguments += [\n> > > > > > > - '-D_FORTIFY_SOURCE=2',\n> > > > > > > - ]\n> > > > > > > + has_fortify_define = false\n> > > > > > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > > > > > + # do the same for c_args.\n> > > > > > > + foreach flag : get_option('cpp_args')\n> > > > > > > + if flag == '-U_FORTIFY_SOURCE'\n> > > > > > > + has_fortify_define = false\n> > > > > >\n> > > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > > false if it was previously been set to true here below ?\n> > > > > >\n> > > > > > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > > > > > false' when the variable is already initialized to 'false'...\n> > > > > >\n> > > > >\n> > > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > > would work equally well for ChromeOS. :)\n> > > >\n> > > > As I read this the code is written to protect against the case where\n> > > > -U_FORTIFY_SOURCE is specified in the same command line -after-\n> > > > -D_FORTIFY_SOURCE, isn't it ?\n> > > >\n> > > > My comment was about the fact `has_fortify_define` is already\n> > > > initialized to false, so there is not need for\n> > > >\n> > > > if flag == '-U_FORTIFY_SOURCE'\n> > > > has_fortify_define = false\n> > > >\n> > > > Unless you don't expect the above described case.\n> > > >\n> > > > As command lines are usually generated by the build system and\n> > > > multiple options to enable/disable a feature might be concatenated in\n> > > > the same line, I'm not opposed to this, but I was wondering if this\n> > > > was by design or not.\n> > >\n> > > Yes, this is by design, and I wrote it this way for the exact reason you\n> > > mention: build flags can be gathered from many different places, and later ones\n> > > take precedence over earlier ones. This logic tries to recognize that and handle\n> > > it appropriately.\n> > >\n> >\n> > Ok, just wanted to make sure this was by design, and I think\n> > protecting against conflicting parameters makes sense!\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n>\n> Thanks again for your help & review! Sorry for my ignorance of kernelish\n> patching procedures, but now that this has been reviewed, is there a process I\n> should follow to get it landed? I don't see my change in the history\n> of https://git.libcamera.org/libcamera/libcamera.git/ yet (HEAD == 90e0fea6c).\n>\n\nNot yet, it needs one more R-b tag and then we can merge it.\n\nAlso, a minor nit, looking at the commit title, I would re-phrase that\nto remove ChromeOS mention from there (there's not CrOS specific, if\nnot that CrOS uses a system-defined command line, but other systems\nmight do the same)\n\nThanks\n j\n\n\n>\n> > > >\n> > > >\n> > > > > > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > > > > > + has_fortify_define = true\n> > > > > > > + endif\n> > > > > > > + endforeach\n> > > > > > > + if not has_fortify_define\n> > > > > > > + common_arguments += [\n> > > > > > > + '-D_FORTIFY_SOURCE=2',\n> > > > > > > + ]\n> > > > > > > + endif\n> > > > > > > endif\n> > > > > > >\n> > > > > > > # Use libc++ by default if available instead of libstdc++ when compiling\n> > > > > > > --\n> > > > > > > 2.42.0.283.g2d96d420d3-goog\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 B62BBBE080\n\tfor ;\n\tTue, 19 Sep 2023 07:08:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 046316293F;\n\tTue, 19 Sep 2023 09:08:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20DB2628D8\n\tfor ;\n\tTue, 19 Sep 2023 09:08:33 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F1678FA2;\n\tTue, 19 Sep 2023 09:06:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695107315;\n\tbh=t3c/CZ4ZNowbIlMJF/5ays97AWdZbTXDA8c9T1BMfJE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=mmCasUMdskc45VM97PMIzzYi8tm0/c10Ifu9oQP8AqOGmE5E+YW/6zvR7F/wUsFEc\n\t6PMVbImkN11aiq2Ut0ibhXC1CzRg4bkj+YgIAoj+CjKnA2PBdLLiMMuLs3zmxl6sD+\n\t4N+R33MTDeGdLtnVvufOn/4zQBK/xJdQjyoULoZnMYFt2OxzyVptTIpzVwhZXqSjTM\n\tTSiyU8QGgU9PYAZllkEpgtZQDcdZSgQlRxr8N/EBDgzt3bAfXfOmgII/hNDm1GSRbv\n\tJ64XxdnbsQM3cLB2arT1wAyV/mSJT4N7gW+PIZh0hVK3iu4+n9/Tq8qouK00m3Bd6g\n\tz/MV8mN425zMA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695107217;\n\tbh=t3c/CZ4ZNowbIlMJF/5ays97AWdZbTXDA8c9T1BMfJE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L3y0i5XPHw8/uXZGeJKDz16lIEM9mLgXCDoo5f0QkSxRzUAs/PQuZg27OQlYJ1mse\n\tB7zZw+PWtq4jF/NJaUORBgZYfGKKRcQjegzAy7q4F4io7A6CIc7aG8YEhUS1tmUTPw\n\t9G0DTVBT+dqLDz8/ygeQ26ZgKRjNEJrAsfdFWjgU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"L3y0i5XP\"; dkim-atps=neutral","Date":"Tue, 19 Sep 2023 09:08:29 +0200","To":"George Burgess ","Message-ID":"","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27817,"web_url":"https://patchwork.libcamera.org/comment/27817/","msgid":"<20230919081923.GC27722@pendragon.ideasonboard.com>","date":"2023-09-19T08:19:23","subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 14, 2023 at 05:32:11PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> On Thu, Sep 14, 2023 at 08:48:49AM -0600, George Burgess via libcamera-devel wrote:\n> > On Thu, Sep 14, 2023 at 1:53 AM Jacopo Mondi wrote:\n> > > On Tue, Sep 12, 2023 at 09:06:20AM -0600, George Burgess via libcamera-devel wrote:\n> > > > Hey,\n> > > >\n> > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > false if it was previously been set to true here below ?\n> > > >\n> > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > would work equally well for ChromeOS. :)\n> > >\n> > > Please do not mangle the email content and try to reply where the\n> > > comment was made in the patch, otherwise it's hard to find context.\n> >\n> > Ah, is replying inline like this the preferred style? I'll try to do that going\n> > forward then - thanks for the tip.\n\nAh, the usual top-posting vs. bottom-posting debate :-)\n\n> > > I'll copy your reply below.\n> > >\n> > > > On Tue, Sep 12, 2023 at 7:22 AM Jacopo Mondi wrote:\n> > > > > On Mon, Sep 11, 2023 at 05:09:07PM -0600, George Burgess IV via libcamera-devel wrote:\n> > > > > > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and\n> > > > > > this definition conflicts with that:\n> > > > > >\n> > > > > > :4:9: error: '_FORTIFY_SOURCE' macro redefined\n> > > > > > [-Werror,-Wmacro-redefined]\n> > > > > >\n> > > > > > Rather than adding logic to keep up with their local configuration, it\n> > > > > > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.\n> > > > > >\n> > > > > > Signed-off-by: George Burgess IV \n> > > > > > ---\n> > > > > > meson.build | 24 +++++++++++++++++++-----\n> > > > > > 1 file changed, 19 insertions(+), 5 deletions(-)\n> > > > > >\n> > > > > > diff --git a/meson.build b/meson.build\n> > > > > > index 7959b538..2e834263 100644\n> > > > > > --- a/meson.build\n> > > > > > +++ b/meson.build\n> > > > > > @@ -99,12 +99,26 @@ if cc.get_id() == 'clang'\n> > > > > > error('clang version is too old, libcamera requires 9.0 or newer')\n> > > > > > endif\n> > > > > >\n> > > > > > - # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1\n> > > > > > - # or higher). This is needed on clang only as gcc enables it by default.\n> > > > > > + # Turn _FORTIFY_SOURCE by default on. This is needed on clang only as gcc\n> > > > > > + # enables it by default. FORTIFY will not work properly with `-O0`, and may\n> > > > > > + # result in macro redefinition errors if the user already has a setting for\n> > > > > > + # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.\n> > > > > > if get_option('optimization') != '0'\n> > > > > > - common_arguments += [\n> > > > > > - '-D_FORTIFY_SOURCE=2',\n> > > > > > - ]\n> > > > > > + has_fortify_define = false\n> > > > > > + # Assume that if the user requests a FORTIFY level in cpp_args, they\n> > > > > > + # do the same for c_args.\n> > > > > > + foreach flag : get_option('cpp_args')\n> > > > > > + if flag == '-U_FORTIFY_SOURCE'\n> > > > > > + has_fortify_define = false\n> > > > >\n> > > > > Do you expect to have -D_FORTIFY_SOURCE and -U_FORTIFY_SOURCE in the\n> > > > > same command line, so that you have to reset has_fortify_define to\n> > > > > false if it was previously been set to true here below ?\n> > > > >\n> > > > > Otherwise I might be missing why you have to set 'has_fortify_define =\n> > > > > false' when the variable is already initialized to 'false'...\n> > > > >\n> > > >\n> > > > You're correct - the code as written intends to answer \"will FORTIFY _be\n> > > > enabled_ by `cpp_args`?\" If you'd like, I'm happy to reduce complexity and have\n> > > > the code determine whether `-D_FORTIFY_SOURCE*` was specified at all. Either\n> > > > would work equally well for ChromeOS. :)\n> > >\n> > > As I read this the code is written to protect against the case where\n> > > -U_FORTIFY_SOURCE is specified in the same command line -after-\n> > > -D_FORTIFY_SOURCE, isn't it ?\n> > >\n> > > My comment was about the fact `has_fortify_define` is already\n> > > initialized to false, so there is not need for\n> > >\n> > > if flag == '-U_FORTIFY_SOURCE'\n> > > has_fortify_define = false\n> > >\n> > > Unless you don't expect the above described case.\n> > >\n> > > As command lines are usually generated by the build system and\n> > > multiple options to enable/disable a feature might be concatenated in\n> > > the same line, I'm not opposed to this, but I was wondering if this\n> > > was by design or not.\n> >\n> > Yes, this is by design, and I wrote it this way for the exact reason you\n> > mention: build flags can be gathered from many different places, and later ones\n> > take precedence over earlier ones. This logic tries to recognize that and handle\n> > it appropriately.\n> \n> Ok, just wanted to make sure this was by design, and I think\n> protecting against conflicting parameters makes sense!\n> \n> Reviewed-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\nBut I'm wondering if we could simplify this by using\ncompiler.get_define(). I'll give it a try on top.\n\n> > > > > > + elif flag.startswith('-D_FORTIFY_SOURCE=')\n> > > > > > + has_fortify_define = true\n> > > > > > + endif\n> > > > > > + endforeach\n> > > > > > + if not has_fortify_define\n> > > > > > + common_arguments += [\n> > > > > > + '-D_FORTIFY_SOURCE=2',\n> > > > > > + ]\n> > > > > > + endif\n> > > > > > endif\n> > > > > >\n> > > > > > # Use libc++ by default if available instead of libstdc++ when compiling","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 5FA75BE080\n\tfor ;\n\tTue, 19 Sep 2023 08:19:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7EBE56293F;\n\tTue, 19 Sep 2023 10:19:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73FE4628D8\n\tfor ;\n\tTue, 19 Sep 2023 10:19:10 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2353EB53;\n\tTue, 19 Sep 2023 10:17:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695111551;\n\tbh=XSumc5jOsqx8x9srjVclaQ+JoHs6x5ZINYjP0WqBfw4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=oGYeNkJk+f1yaTs/LDmNaYemqtNhYnkW4xkyn6Y5SIDE7aDlhE/Dh/Eprk055n/8X\n\tvRWF4BVYTpxnMreFsg2zKhKFj+NUYcDY8mCY9W5ZlxLKxMDjhDQVdp75U9CfkKDjnD\n\tN583Y1r2NUcuS9ylv6HJDqj4I2n0t83rnz2W/7Wdk6ih6xeKccO+81nQFsMdWRMhoi\n\tTbylyUvzt3OdLzTRNqToR8OcASY6n/1NflBCi6H2bhsqQ1+Q0G1hnBUYhzuZLBf0Op\n\tEHWlo46h1KIlvpI37t6QlCxAsiv+oBCMqLMxixZn2RAfi5hEunNoZr46PpzC0+Mz6P\n\tdecVBtDXvqG9w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695111454;\n\tbh=XSumc5jOsqx8x9srjVclaQ+JoHs6x5ZINYjP0WqBfw4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YzihcP4yFXU66ZV+uglbaLu6A9Wdv0idwQ7tnXG23tZrpHMWG28b0ja8dgSOSdEW2\n\tuvXVvM0xsl8g5zIZAcU2Hd/ha40Kt/p9VK2/4vCbhPWF04+xigr+jeOIyi/l4nQxTN\n\tNJGreSq560XT17l6tdN8wfUoIZtmEwfVlTDFOSMY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YzihcP4y\"; dkim-atps=neutral","Date":"Tue, 19 Sep 2023 11:19:23 +0300","To":"Jacopo Mondi ","Message-ID":"<20230919081923.GC27722@pendragon.ideasonboard.com>","References":"<20230911230909.1263308-1-gbiv@google.com>\n\t<6byqf3ocgj6nxj3tnhrpsmvn4cir6il6ynlkhcs7isdvzhh5sr@ztqnsxkfufya>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2] meson: Don't set _FORTIFY_SOURCE\n\tfor ChromeOS","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, George Burgess ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]