Re: CVS commit: src/external/cddl/osnet/lib/libdtrace

2020-03-16 Thread Santhosh Raju
On Mon, Mar 16, 2020 at 8:18 PM Santhosh Raju  wrote:
>
> On Mon, Mar 16, 2020 at 8:02 PM Christos Zoulas  wrote:
> >
> > In article <20200317005012.daf4af...@cvs.netbsd.org>,
> > Santhosh Raju  wrote:
> > >-=-=-=-=-=-
> > >
> > >Module Name:   src
> > >Committed By:  fox
> > >Date:  Tue Mar 17 00:50:12 UTC 2020
> > >
> > >Modified Files:
> > >   src/external/cddl/osnet/lib/libdtrace: Makefile
> > >
> > >Log Message:
> > >external/cddl/osnet: Supress -Werror=maybe-uninitialized error in 
> > >libdtrace.
> > >
> > >It looks like this is a false positive, since the section of code
> > >triggering the error
> > >
> > >external/cddl/osnet/dist/lib/libdtrace/common/dt_proc.c:400:42:
> > >
> > >is only accessed after "err" is initialized.
> > >
> > >Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag.
> >
> > You did not just suppress the error; you suppressed the warning too...
> > There is a difference between -Wno-error=maybe-uninitialized and
> > -Wno-maybe-uninitialized. I think we want the first flavor, otherwise
> > this is a large axe that will hide other warnings in the long run.
> >
>
> Agreed, I shall make the change to be -Wno-error=maybe-uninitialized.
>

https://mail-index.netbsd.org/source-changes/2020/03/17/msg115173.html

Should be fixed in this commit.

--
Santhosh


Re: CVS commit: src/external/cddl/osnet/lib/libdtrace

2020-03-16 Thread Santhosh Raju
On Mon, Mar 16, 2020 at 8:02 PM Christos Zoulas  wrote:
>
> In article <20200317005012.daf4af...@cvs.netbsd.org>,
> Santhosh Raju  wrote:
> >-=-=-=-=-=-
> >
> >Module Name:   src
> >Committed By:  fox
> >Date:  Tue Mar 17 00:50:12 UTC 2020
> >
> >Modified Files:
> >   src/external/cddl/osnet/lib/libdtrace: Makefile
> >
> >Log Message:
> >external/cddl/osnet: Supress -Werror=maybe-uninitialized error in libdtrace.
> >
> >It looks like this is a false positive, since the section of code
> >triggering the error
> >
> >external/cddl/osnet/dist/lib/libdtrace/common/dt_proc.c:400:42:
> >
> >is only accessed after "err" is initialized.
> >
> >Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag.
>
> You did not just suppress the error; you suppressed the warning too...
> There is a difference between -Wno-error=maybe-uninitialized and
> -Wno-maybe-uninitialized. I think we want the first flavor, otherwise
> this is a large axe that will hide other warnings in the long run.
>

Agreed, I shall make the change to be -Wno-error=maybe-uninitialized.

> Now perhaps it is better to do what we've been doing traditionallly:
> over-initialize the variable with 'foo = 0; // XXX: gcc', but that's
> more book-keeping (but at least it is localized as opposed to suppress
> for the entire compilation unit). Please note that we don't have a good
> way to go around and test those error-avoidance overrides each time we
> upgrade the compiler so they tend to stick forever.
>

I did think of over-initialize here, but I did not know if it would
change the desired behavior of the code.

> christos1
>

--
Santhosh


Re: CVS commit: src/external/cddl/osnet/lib/libdtrace

2020-03-16 Thread Christos Zoulas
In article <20200317005012.daf4af...@cvs.netbsd.org>,
Santhosh Raju  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  fox
>Date:  Tue Mar 17 00:50:12 UTC 2020
>
>Modified Files:
>   src/external/cddl/osnet/lib/libdtrace: Makefile
>
>Log Message:
>external/cddl/osnet: Supress -Werror=maybe-uninitialized error in libdtrace.
>
>It looks like this is a false positive, since the section of code
>triggering the error
>
>external/cddl/osnet/dist/lib/libdtrace/common/dt_proc.c:400:42:
>
>is only accessed after "err" is initialized.
>
>Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag.

You did not just suppress the error; you suppressed the warning too...
There is a difference between -Wno-error=maybe-uninitialized and
-Wno-maybe-uninitialized. I think we want the first flavor, otherwise
this is a large axe that will hide other warnings in the long run.

Now perhaps it is better to do what we've been doing traditionallly:
over-initialize the variable with 'foo = 0; // XXX: gcc', but that's
more book-keeping (but at least it is localized as opposed to suppress
for the entire compilation unit). Please note that we don't have a good
way to go around and test those error-avoidance overrides each time we
upgrade the compiler so they tend to stick forever.

christos1