| Message ID | 20201013151241.3557005-11-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, On 2020-10-13 16:12:41 +0100, Kieran Bingham wrote: > Shadowing variables can lead to unexpected bugs where a code path > utilises a variable that may not have been intended by the developer, > leading to hard to find bugs. > > Enable warnings for shadowed variables as defined at: > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow > > As an effect of utilising -Werror, this will cause variable or type > shadowing to become a build-time error. > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> I like it! Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > The reference here is that I lost around 2 hours digging into a bug that > looked like some other code path was calling a destructor or such, but > was because I had accidentally aliased a private member variable in the > constructor, so all my initialisation was lost after the constructor > completed. > > meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/meson.build b/meson.build > index de15cc16da81..10423f523ca5 100644 > --- a/meson.build > +++ b/meson.build > @@ -38,6 +38,7 @@ if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOUR > endif > > common_arguments = [ > + '-Wshadow', > '-include', 'config.h', > ] > > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, Thank you for the patch. On Tue, Oct 13, 2020 at 04:12:41PM +0100, Kieran Bingham wrote: > Shadowing variables can lead to unexpected bugs where a code path > utilises a variable that may not have been intended by the developer, > leading to hard to find bugs. > > Enable warnings for shadowed variables as defined at: > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow > > As an effect of utilising -Werror, this will cause variable or type > shadowing to become a build-time error. > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > The reference here is that I lost around 2 hours digging into a bug that > looked like some other code path was calling a destructor or such, but > was because I had accidentally aliased a private member variable in the > constructor, so all my initialisation was lost after the constructor > completed. It's sometimes nice to be able to shadow variables, but I think the debugging pain is too high of a cost for that. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/meson.build b/meson.build > index de15cc16da81..10423f523ca5 100644 > --- a/meson.build > +++ b/meson.build > @@ -38,6 +38,7 @@ if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOUR > endif > > common_arguments = [ > + '-Wshadow', > '-include', 'config.h', > ]
diff --git a/meson.build b/meson.build index de15cc16da81..10423f523ca5 100644 --- a/meson.build +++ b/meson.build @@ -38,6 +38,7 @@ if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOUR endif common_arguments = [ + '-Wshadow', '-include', 'config.h', ]
Shadowing variables can lead to unexpected bugs where a code path utilises a variable that may not have been intended by the developer, leading to hard to find bugs. Enable warnings for shadowed variables as defined at: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow As an effect of utilising -Werror, this will cause variable or type shadowing to become a build-time error. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- The reference here is that I lost around 2 hours digging into a bug that looked like some other code path was calling a destructor or such, but was because I had accidentally aliased a private member variable in the constructor, so all my initialisation was lost after the constructor completed. meson.build | 1 + 1 file changed, 1 insertion(+)