[libcamera-devel] [PATCH] meson: Don't set _FORTIFY_SOURCE for ChromeOS

[George Burgess]

Thank you both for the quick feedback!

> I think we can drop the ``` ?

Done

> Not a native speaker here but, is "sets" and "their" in the same
> phrase ok ? Shouldn't this be either ("set" and "their") or ("sets" and
> "its") ?

Yeah, I agree the wording here was strange on my part. Fixed.

> Minor nitpicking apart, I think the patch is fine. Just curious to
> know if switching to FORTIFY_SOURCE=3 will generate new bug reports :)

Maybe? :) The switch has been pretty clean for us so far.

> Someone else may do the same. I wonder, would it be possible here to add
> -D_FORTIFY_SOURCES only if not already set in CFLAGS ?

ChromeOS is... a bit weird. Our ${CC}, ${CXX}, etc are actually wrappers that
apply global CFLAGS (among other things). This makes it pretty hard to detect
the complete set of flags are actually being set for a given build.

*That said*, I can probably add a nop -D_FORTIFY_SOURCE=3 to the actual
${CFLAGS} for this package in ChromeOS, so this change can be made more
generally applicable. I'll send v2 once I get the chance to test that.

George

On Fri, Sep 8, 2023 at 9:08 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Thu, Sep 07, 2023 at 10:12:59AM -0600, George Burgess IV via libcamera-devel wrote:
> > ChromeOS is moving to a platform default of `_FORTIFY_SOURCE=3`, and
> > this definition conflicts with that:
> >
> > ```
> > <command line>:4:9: error: '_FORTIFY_SOURCE' macro redefined
> > [-Werror,-Wmacro-redefined]
> > ```
> >
> > Rather than adding logic to keep up with their local configuration, it
> > seems best to leave setting _FORTIFY_SOURCE on ChromeOS up to ChromeOS.
> >
> > Signed-off-by: George Burgess IV <gbiv at google.com>
> > ---
> >  meson.build | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 7959b538..109923ac 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -101,7 +101,8 @@ if cc.get_id() == 'clang'
> >
> >      # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1
> >      # or higher). This is needed on clang only as gcc enables it by default.
> > -    if get_option('optimization') != '0'
> > +    # CrOS sets their preferred FORTIFY level in platform-level CFLAGS.
>
> Someone else may do the same. I wonder, would it be possible here to add
> -D_FORTIFY_SOURCES only if not already set in CFLAGS ?
>
> > +    if get_option('optimization') != '0' and get_option('android_platform') != 'cros'
> >          common_arguments += [
> >              '-D_FORTIFY_SOURCE=2',
> >          ]
>
> --
> Regards,
>
> Laurent Pinchart