| Message ID | 20190819130245.7298-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2019-08-19 16:02:45 +0300, Laurent Pinchart wrote: > _FORTIFY_SOURCE add useful checks during compilation. Enable it by > default, except for debug builds as it requires at least a -O1 > optimisation level. I think this is a good idea, it's a shame we can't have it enabled for debug builds. My normal workflow of "meson build; ninja -C build" produces debug builds so if this catch a problem it will be at release time. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > meson.build | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/meson.build b/meson.build > index 13d0605f903c..53263d68791d 100644 > --- a/meson.build > +++ b/meson.build > @@ -35,6 +35,12 @@ common_arguments = [ > '-include', 'config.h', > ] > > +if get_option('buildtype') != 'debug' > + common_arguments += [ > + '-D_FORTIFY_SOURCE=1', > + ] > +endif > + > c_arguments = common_arguments > cpp_arguments = common_arguments > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Mon, Aug 26, 2019 at 11:45:33PM +0200, Niklas Söderlund wrote: > On 2019-08-19 16:02:45 +0300, Laurent Pinchart wrote: > > _FORTIFY_SOURCE add useful checks during compilation. Enable it by > > default, except for debug builds as it requires at least a -O1 > > optimisation level. > > I think this is a good idea, it's a shame we can't have it enabled for > debug builds. My normal workflow of "meson build; ninja -C build" > produces debug builds so if this catch a problem it will be at release > time. We could change the default build type too. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > --- > > meson.build | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/meson.build b/meson.build > > index 13d0605f903c..53263d68791d 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -35,6 +35,12 @@ common_arguments = [ > > '-include', 'config.h', > > ] > > > > +if get_option('buildtype') != 'debug' > > + common_arguments += [ > > + '-D_FORTIFY_SOURCE=1', > > + ] > > +endif > > + > > c_arguments = common_arguments > > cpp_arguments = common_arguments > >
Hi Laurent, On 2019-08-27 00:51:44 +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Mon, Aug 26, 2019 at 11:45:33PM +0200, Niklas Söderlund wrote: > > On 2019-08-19 16:02:45 +0300, Laurent Pinchart wrote: > > > _FORTIFY_SOURCE add useful checks during compilation. Enable it by > > > default, except for debug builds as it requires at least a -O1 > > > optimisation level. > > > > I think this is a good idea, it's a shame we can't have it enabled for > > debug builds. My normal workflow of "meson build; ninja -C build" > > produces debug builds so if this catch a problem it will be at release > > time. > > We could change the default build type too. That is one option, another is that I change my workflow to also build and test release builds. I think there is value in keeping debug builds as the default. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > --- > > > meson.build | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/meson.build b/meson.build > > > index 13d0605f903c..53263d68791d 100644 > > > --- a/meson.build > > > +++ b/meson.build > > > @@ -35,6 +35,12 @@ common_arguments = [ > > > '-include', 'config.h', > > > ] > > > > > > +if get_option('buildtype') != 'debug' > > > + common_arguments += [ > > > + '-D_FORTIFY_SOURCE=1', > > > + ] > > > +endif > > > + > > > c_arguments = common_arguments > > > cpp_arguments = common_arguments > > > > > -- > Regards, > > Laurent Pinchart
Hi Laurent, On 26/08/2019 22:51, Laurent Pinchart wrote: > Hi Niklas, > > On Mon, Aug 26, 2019 at 11:45:33PM +0200, Niklas Söderlund wrote: >> On 2019-08-19 16:02:45 +0300, Laurent Pinchart wrote: >>> _FORTIFY_SOURCE add useful checks during compilation. Enable it by >>> default, except for debug builds as it requires at least a -O1 >>> optimisation level. >> >> I think this is a good idea, it's a shame we can't have it enabled for >> debug builds. My normal workflow of "meson build; ninja -C build" >> produces debug builds so if this catch a problem it will be at release >> time. > > We could change the default build type too. I think we should keep our default build type as debug. During development we want debug symbols and any extra checks such as fail fast assertions. Yes it's annoying that we can't have /these/ extra checks at build time in debug too - but release builds will be built ... Building as release should be done by automated build processes. (which Mauro has already started) so should be caught there. So for this, we just need to make sure Mauro's build is configured as a release build. Or better yet, has multiple build configurations. >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> >> >>> --- >>> meson.build | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/meson.build b/meson.build >>> index 13d0605f903c..53263d68791d 100644 >>> --- a/meson.build >>> +++ b/meson.build >>> @@ -35,6 +35,12 @@ common_arguments = [ >>> '-include', 'config.h', >>> ] >>> >>> +if get_option('buildtype') != 'debug' >>> + common_arguments += [ >>> + '-D_FORTIFY_SOURCE=1', >>> + ] >>> +endif >>> + >>> c_arguments = common_arguments >>> cpp_arguments = common_arguments >>>
Hi Kieran, On Tue, Aug 27, 2019 at 08:47:41AM +0100, Kieran Bingham wrote: > On 26/08/2019 22:51, Laurent Pinchart wrote: > > On Mon, Aug 26, 2019 at 11:45:33PM +0200, Niklas Söderlund wrote: > >> On 2019-08-19 16:02:45 +0300, Laurent Pinchart wrote: > >>> _FORTIFY_SOURCE add useful checks during compilation. Enable it by > >>> default, except for debug builds as it requires at least a -O1 > >>> optimisation level. > >> > >> I think this is a good idea, it's a shame we can't have it enabled for > >> debug builds. My normal workflow of "meson build; ninja -C build" > >> produces debug builds so if this catch a problem it will be at release > >> time. > > > > We could change the default build type too. > > I think we should keep our default build type as debug. During > development we want debug symbols and any extra checks such as fail fast > assertions. Yes it's annoying that we can't have /these/ extra checks at > build time in debug too - but release builds will be built ... debug builds for development makes sense, but a different default build type might still make sense. What's the most common case, libcamera development, or libcamera packaging ? For us it's clearly development :-) I think we can postpone any change to the default build type, but at some point we may want to reconsider, if only to avoid distribution incorrectly packaging a debug build. > Building as release should be done by automated build processes. > (which Mauro has already started) so should be caught there. > > So for this, we just need to make sure Mauro's build is configured as a > release build. Or better yet, has multiple build configurations. > > >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > >> > >>> --- > >>> meson.build | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/meson.build b/meson.build > >>> index 13d0605f903c..53263d68791d 100644 > >>> --- a/meson.build > >>> +++ b/meson.build > >>> @@ -35,6 +35,12 @@ common_arguments = [ > >>> '-include', 'config.h', > >>> ] > >>> > >>> +if get_option('buildtype') != 'debug' > >>> + common_arguments += [ > >>> + '-D_FORTIFY_SOURCE=1', > >>> + ] > >>> +endif > >>> + > >>> c_arguments = common_arguments > >>> cpp_arguments = common_arguments > >>>
diff --git a/meson.build b/meson.build index 13d0605f903c..53263d68791d 100644 --- a/meson.build +++ b/meson.build @@ -35,6 +35,12 @@ common_arguments = [ '-include', 'config.h', ] +if get_option('buildtype') != 'debug' + common_arguments += [ + '-D_FORTIFY_SOURCE=1', + ] +endif + c_arguments = common_arguments cpp_arguments = common_arguments
_FORTIFY_SOURCE add useful checks during compilation. Enable it by default, except for debug builds as it requires at least a -O1 optimisation level. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- meson.build | 6 ++++++ 1 file changed, 6 insertions(+)