[libcamera-devel] [PATCH] meson: Really define _FORTIFY_SOURCE for optimised builds

[Laurent Pinchart]

Hi Kieran,

On Thu, Nov 21, 2019 at 09:17:50AM +0000, Kieran Bingham wrote:
> On 21/11/2019 04:19, Laurent Pinchart wrote:
> > Commit 965c5bf7fbf5 ("meson: Define _FORTIFY_SOURCE for optimised
> > builds") tried to define _FORTIFY_SOURCE for optimised builds with
> > clang, but updated the common_arguments after it was used. This resulted
> > in the _FORTIFY_SOURCE option not being applied. Fix it.
> > 
> > Fixes: 965c5bf7fbf5 ("meson: Define _FORTIFY_SOURCE for optimised builds")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  meson.build | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 72ad7c8b493b..0a222ba96dcb 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -35,8 +35,8 @@ common_arguments = [
> >      '-include', 'config.h',
> >  ]
> >  
> > -c_arguments = common_arguments
> > -cpp_arguments = common_arguments
> > +c_arguments = []
> > +cpp_arguments = []
> 
> c_arguments and cpp_arguments are not used until after the code block
> below. Do we really need to define them as empty arrays here?
> 
> I /think/ we can just move the assignment to below?
>  (where you do an addition instead)

I could do that for cpp_arguments, but cpp_arguments is extended in the
if cc.get_id() == 'clang' block. I could assign it there, but then the
+= might complain later. I'd rather keep an empty array at the beginning
and += everywhere to make it easily extensible and avoid future issues.

> >  if cc.get_id() == 'clang'
> >      # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1
> > @@ -56,6 +56,9 @@ if cc.get_id() == 'clang'
> >      endif
> >  endif
> >  
> > +c_arguments += common_arguments
> > +cpp_arguments += common_arguments
> > +>  add_project_arguments(c_arguments, language : 'c')
> 
> Eeep - and that's so blindingly obvious when you look at 965c5bf7fbf5,
> as this line is clearly in the hunk. I'm sorry I missed it.
> 
> (of course it's always easier to spot something that you know is there).
> 
> With either the empty arrays declared above if that's necessary (or just
> preferred) or directly assigned lower down if it's appropriate:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >  add_project_arguments(cpp_arguments, language : 'cpp')
> >  add_project_link_arguments(cpp_arguments, language : 'cpp')
> > 

-- 
Regards,

Laurent Pinchart