[libcamera-devel] meson: Simplify check for _FORTIFY_SOURCE
diff mbox series

Message ID 20230919133943.32035-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 1d616141420d1f51e5999d758e3e0cc721a46290
Headers show
Series
  • [libcamera-devel] meson: Simplify check for _FORTIFY_SOURCE
Related show

Commit Message

Laurent Pinchart Sept. 19, 2023, 1:39 p.m. UTC 
Use the compiler.get_define() function to get the value of
_FORTIFY_SOURCE instead of iterating over the cpp_args. This simplies
the code, but also guarantees to return the actual value of
_FORTIFY_SOURCE, even if defined through other means than through the
meson cpp_args.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 meson.build | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)


base-commit: 9c5eb9237cf6ae170086f0d4d87a025aa052cc9f

Comments

George Burgess Sept. 19, 2023, 5:07 p.m. UTC | #1 
On Tue, Sep 19, 2023 at 7:39 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Use the compiler.get_define() function to get the value of
> _FORTIFY_SOURCE instead of iterating over the cpp_args. This simplies
> the code, but also guarantees to return the actual value of
> _FORTIFY_SOURCE, even if defined through other means than through the
> meson cpp_args.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  meson.build | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 2e8342630332..e9a1c7e360ce 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -104,17 +104,9 @@ if cc.get_id() == 'clang'
>      # result in macro redefinition errors if the user already has a setting for
>      # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.
>      if get_option('optimization') != '0'
> -        has_fortify_define = false
> -        # Assume that if the user requests a FORTIFY level in cpp_args, they
> -        # do the same for c_args.
> -        foreach flag : get_option('cpp_args')
> -            if flag == '-U_FORTIFY_SOURCE'
> -                has_fortify_define = false
> -            elif flag.startswith('-D_FORTIFY_SOURCE=')
> -                has_fortify_define = true
> -            endif
> -        endforeach
> -        if not has_fortify_define
> +        fortify = cc.get_define('_FORTIFY_SOURCE')

TIL about `cc.get_define` - nice cleanup :)

I've tested this in CrOS and it works just as well, so in case my
review is helpful...

Reviewed-by: George Burgess IV <gbiv@google.com>

Thanks!

> +        if fortify == ''
> +            message('Adding _FORTIFY_SOURCE')
>              common_arguments += [
>                  '-D_FORTIFY_SOURCE=2',
>              ]
>
> base-commit: 9c5eb9237cf6ae170086f0d4d87a025aa052cc9f
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart Sept. 19, 2023, 6:38 p.m. UTC | #2 
On Tue, Sep 19, 2023 at 11:07:55AM -0600, George Burgess wrote:
> On Tue, Sep 19, 2023 at 7:39 AM Laurent Pinchart wrote:
> >
> > Use the compiler.get_define() function to get the value of
> > _FORTIFY_SOURCE instead of iterating over the cpp_args. This simplies
> > the code, but also guarantees to return the actual value of
> > _FORTIFY_SOURCE, even if defined through other means than through the
> > meson cpp_args.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  meson.build | 14 +++-----------
> >  1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 2e8342630332..e9a1c7e360ce 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -104,17 +104,9 @@ if cc.get_id() == 'clang'
> >      # result in macro redefinition errors if the user already has a setting for
> >      # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.
> >      if get_option('optimization') != '0'
> > -        has_fortify_define = false
> > -        # Assume that if the user requests a FORTIFY level in cpp_args, they
> > -        # do the same for c_args.
> > -        foreach flag : get_option('cpp_args')
> > -            if flag == '-U_FORTIFY_SOURCE'
> > -                has_fortify_define = false
> > -            elif flag.startswith('-D_FORTIFY_SOURCE=')
> > -                has_fortify_define = true
> > -            endif
> > -        endforeach
> > -        if not has_fortify_define
> > +        fortify = cc.get_define('_FORTIFY_SOURCE')
> 
> TIL about `cc.get_define` - nice cleanup :)
> 
> I've tested this in CrOS and it works just as well, so in case my
> review is helpful...
> 
> Reviewed-by: George Burgess IV <gbiv@google.com>

Thank you. It's more than helpful, given that this problem mainly
affects you :-)

> > +        if fortify == ''
> > +            message('Adding _FORTIFY_SOURCE')
> >              common_arguments += [
> >                  '-D_FORTIFY_SOURCE=2',
> >              ]
> >
> > base-commit: 9c5eb9237cf6ae170086f0d4d87a025aa052cc9f
Jacopo Mondi Sept. 21, 2023, 10:41 a.m. UTC | #3 
Hi Laurent

On Tue, Sep 19, 2023 at 11:07:55AM -0600, George Burgess via libcamera-devel wrote:
> On Tue, Sep 19, 2023 at 7:39 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Use the compiler.get_define() function to get the value of
> > _FORTIFY_SOURCE instead of iterating over the cpp_args. This simplies
> > the code, but also guarantees to return the actual value of
> > _FORTIFY_SOURCE, even if defined through other means than through the
> > meson cpp_args.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  meson.build | 14 +++-----------
> >  1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 2e8342630332..e9a1c7e360ce 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -104,17 +104,9 @@ if cc.get_id() == 'clang'
> >      # result in macro redefinition errors if the user already has a setting for
> >      # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.
> >      if get_option('optimization') != '0'
> > -        has_fortify_define = false
> > -        # Assume that if the user requests a FORTIFY level in cpp_args, they
> > -        # do the same for c_args.
> > -        foreach flag : get_option('cpp_args')
> > -            if flag == '-U_FORTIFY_SOURCE'
> > -                has_fortify_define = false
> > -            elif flag.startswith('-D_FORTIFY_SOURCE=')
> > -                has_fortify_define = true
> > -            endif
> > -        endforeach
> > -        if not has_fortify_define
> > +        fortify = cc.get_define('_FORTIFY_SOURCE')
>
> TIL about `cc.get_define` - nice cleanup :)

same here!

>
> I've tested this in CrOS and it works just as well, so in case my
> review is helpful...
>
> Reviewed-by: George Burgess IV <gbiv@google.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j
>
> Thanks!
>
> > +        if fortify == ''
> > +            message('Adding _FORTIFY_SOURCE')
> >              common_arguments += [
> >                  '-D_FORTIFY_SOURCE=2',
> >              ]
> >
> > base-commit: 9c5eb9237cf6ae170086f0d4d87a025aa052cc9f
> > --
> > Regards,
> >
> > Laurent Pinchart
> >

Patch
diff mbox series 

diff --git a/meson.build b/meson.build
index 2e8342630332..e9a1c7e360ce 100644
--- a/meson.build
+++ b/meson.build
@@ -104,17 +104,9 @@  if cc.get_id() == 'clang'
     # result in macro redefinition errors if the user already has a setting for
     # `-D_FORTIFY_SOURCE`. Do not enable FORTIFY in either of those cases.
     if get_option('optimization') != '0'
-        has_fortify_define = false
-        # Assume that if the user requests a FORTIFY level in cpp_args, they
-        # do the same for c_args.
-        foreach flag : get_option('cpp_args')
-            if flag == '-U_FORTIFY_SOURCE'
-                has_fortify_define = false
-            elif flag.startswith('-D_FORTIFY_SOURCE=')
-                has_fortify_define = true
-            endif
-        endforeach
-        if not has_fortify_define
+        fortify = cc.get_define('_FORTIFY_SOURCE')
+        if fortify == ''
+            message('Adding _FORTIFY_SOURCE')
             common_arguments += [
                 '-D_FORTIFY_SOURCE=2',
             ]