Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
>
> False positives are compiler bugs.

No they're not. No more than missed optimization opportunities.
They're limitations in the control flow analysis.

>
> It does happen, with GCC more than with clang, that the compiler has too
> many bugs and it's a bad practice to pessimize code to work around them.

It doesn't pessimize the compiled output. It may make the code less
readable in the opinion of some.  I've found that 10% of the warnings
were actually legitimate issues. It's unfortunate that the majority
are just noise that have to be waded through.

>  At
> very least you should add a comment when adding unnecessary initializations,
> something like /* workaround GCC */, but dropping broken warnings is best.

That's legitimate.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Pedro Giffuni 


On 05/24/18 11:00, Matthew Macy wrote:

On Thu, May 24, 2018 at 8:58 AM, Warner Losh  wrote:


On Thu, May 24, 2018 at 12:53 AM, Matthew Macy  wrote:

On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
 wrote:

On 24. May 2018, at 08:36, Matthew Macy  wrote:

On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
 wrote:

On 24. May 2018, at 06:51, Matthew Macy  wrote:

Warnings find bugs PERIOD. Although most are not useful, I've found

Some warnings indicate bugs, some warnings are just wrong. If you
have a "may be used uninitialized" warning being a false positive, you
may silences the warning by just set it to zero in the declaration and
you silence it. Other compilers might then correctly report an
assignment without affect...

I have yet to see a double assignment be flagged as assignment without
effect. If it _does_ occur then we have to disable the warning on the
compiler that we have less faith in.

Have seen it in the past in a difference project... But you miss my
point:

Not all warnings indicate bugs PERIOD. Some warning are just wrong...

Have you read my follow up? _Many_ Many warnings are wrong. Please
respond to that on what the global policy should be. The value of any
one particular instance of a warning does not merit discussion.


The global policy has never been 'fix all warnings no matter what.' It's
been 'Look at the warning. If it's a false positive, use judgement about
whether or not to stifle the compiler.' There are cases I've run into that
it was impossible to silence the warnings (apart form adding command line
stuff) for a particular bit of code. Do it one way gcc 4.2 complains. Do it
another clang complains. appease both and gcc 6 had heart-burn.

So don't gratuitously commit code that fixes warnings on gcc 8. If the
warning points out a legitimate bug, then that's no brainer yes. If it's a
false positive, then it's less clear and often times many factors may need
to be weighed.

Non-actionable warnings are actively detrimental to workflow. They
hide real issues and lead to apathy by developers. If pacifying a
warning is considered undesirable it should be disabled by default
with perhaps a separate mode for enabling it.


False positives are compiler bugs.

It does happen, with GCC more than with clang, that the compiler has too 
many bugs and it's a bad practice to pessimize code to work around 
them.  At very least you should add a comment when adding unnecessary 
initializations, something like /* workaround GCC */, but dropping 
broken warnings is best.


Pedro.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Thu, May 24, 2018 at 8:58 AM, Warner Losh  wrote:
>
>
> On Thu, May 24, 2018 at 12:53 AM, Matthew Macy  wrote:
>>
>> On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
>>  wrote:
>> >> On 24. May 2018, at 08:36, Matthew Macy  wrote:
>> >>
>> >> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>> >>  wrote:
>>  On 24. May 2018, at 06:51, Matthew Macy  wrote:
>> 
>>  Warnings find bugs PERIOD. Although most are not useful, I've found
>> >>> Some warnings indicate bugs, some warnings are just wrong. If you
>> >>> have a "may be used uninitialized" warning being a false positive, you
>> >>> may silences the warning by just set it to zero in the declaration and
>> >>> you silence it. Other compilers might then correctly report an
>> >>> assignment without affect...
>> >>
>> >> I have yet to see a double assignment be flagged as assignment without
>> >> effect. If it _does_ occur then we have to disable the warning on the
>> >> compiler that we have less faith in.
>> > Have seen it in the past in a difference project... But you miss my
>> > point:
>> >
>> > Not all warnings indicate bugs PERIOD. Some warning are just wrong...
>>
>> Have you read my follow up? _Many_ Many warnings are wrong. Please
>> respond to that on what the global policy should be. The value of any
>> one particular instance of a warning does not merit discussion.
>
>
> The global policy has never been 'fix all warnings no matter what.' It's
> been 'Look at the warning. If it's a false positive, use judgement about
> whether or not to stifle the compiler.' There are cases I've run into that
> it was impossible to silence the warnings (apart form adding command line
> stuff) for a particular bit of code. Do it one way gcc 4.2 complains. Do it
> another clang complains. appease both and gcc 6 had heart-burn.
>
> So don't gratuitously commit code that fixes warnings on gcc 8. If the
> warning points out a legitimate bug, then that's no brainer yes. If it's a
> false positive, then it's less clear and often times many factors may need
> to be weighed.

Non-actionable warnings are actively detrimental to workflow. They
hide real issues and lead to apathy by developers. If pacifying a
warning is considered undesirable it should be disabled by default
with perhaps a separate mode for enabling it.

-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Warner Losh 
On Thu, May 24, 2018 at 12:53 AM, Matthew Macy  wrote:

> On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
>  wrote:
> >> On 24. May 2018, at 08:36, Matthew Macy  wrote:
> >>
> >> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
> >>  wrote:
>  On 24. May 2018, at 06:51, Matthew Macy  wrote:
> 
>  Warnings find bugs PERIOD. Although most are not useful, I've found
> >>> Some warnings indicate bugs, some warnings are just wrong. If you
> >>> have a "may be used uninitialized" warning being a false positive, you
> >>> may silences the warning by just set it to zero in the declaration and
> >>> you silence it. Other compilers might then correctly report an
> >>> assignment without affect...
> >>
> >> I have yet to see a double assignment be flagged as assignment without
> >> effect. If it _does_ occur then we have to disable the warning on the
> >> compiler that we have less faith in.
> > Have seen it in the past in a difference project... But you miss my
> point:
> >
> > Not all warnings indicate bugs PERIOD. Some warning are just wrong...
>
> Have you read my follow up? _Many_ Many warnings are wrong. Please
> respond to that on what the global policy should be. The value of any
> one particular instance of a warning does not merit discussion.
>

The global policy has never been 'fix all warnings no matter what.' It's
been 'Look at the warning. If it's a false positive, use judgement about
whether or not to stifle the compiler.' There are cases I've run into that
it was impossible to silence the warnings (apart form adding command line
stuff) for a particular bit of code. Do it one way gcc 4.2 complains. Do it
another clang complains. appease both and gcc 6 had heart-burn.

So don't gratuitously commit code that fixes warnings on gcc 8. If the
warning points out a legitimate bug, then that's no brainer yes. If it's a
false positive, then it's less clear and often times many factors may need
to be weighed.

Warner


> -M
>
>
>
>
>
>
>
>
>
>
>
> , in this case the assignment should be added with
> > a comment /* pacify gcc */.
> >
> > On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> > M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff <
> gleb...@freebsd.org> wrote:
> > M> > The initialization isn't useful.
> > M>
> > M> It silences a gcc warning. So yes it is. It's this exchange which
> is not useful.
> > M>
> > M> -M
> > M>
> > M>
> > M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> > M> > M> Talk to the gcc devs. The warning is useful even if there
> are false positives.
> > M> > M>
> > M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff <
> gleb...@freebsd.org> wrote:
> > M> > M> >   Hi,
> > M> > M> >
> > M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> > M> > M> > M> Author: mmacy
> > M> > M> > M> Date: Sat May 19 05:10:51 2018
> > M> > M> > M> New Revision: 333860
> > M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> > M> > M> > M>
> > M> > M> > M> Log:
> > M> > M> > M>   sendfile: annotate unused value and ensure that
> npages is actually initialized
> > M> > M> > M>
> > M> > M> > M> Modified:
> > M> > M> > M>   head/sys/kern/kern_sendfile.c
> > M> > M> > M>
> > M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> > M> > M> > M> ==
> 
> > M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19
> 05:09:10 2018(r333859)
> > M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19
> 05:10:51 2018(r333860)
> > M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj,
> struct sf_io *sfio, o
> > M> > M> > M>  }
> > M> > M> > M>
> > M> > M> > M>  for (int i = 0; i < npages;) {
> > M> > M> > M> -int j, a, count, rv;
> > M> > M> > M> +int j, a, count, rv __unused;
> > M> > M> > M>
> > M> > M> > M>  /* Skip valid pages. */
> > M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off)
> & PAGE_MASK,
> > M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> > M> > M> > M>  if (space == 0) {
> > M> > M> > M>  sfio = NULL;
> > M> > M> > M>  nios = 0;
> > M> > M> > M> +npages = 0;
> > M> > M> > M>  goto prepend_header;
> > M> > M> > M>  }
> > M> > M> > M>  hdr_uio = NULL;
> > M> > M> >
> > M> > M> > This initialization is redundant and a compiler warning if
> exists is wrong.
> > M> > M> >
> > M> > M> > If we jump down to prepend_header with nios == 0, we won't
> ever

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Thu, May 24, 2018 at 8:54 AM, Warner Losh  wrote:
>
>
> On Thu, May 24, 2018 at 12:36 AM, Matthew Macy  wrote:
>>
>> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>>  wrote:
>> >> On 24. May 2018, at 06:51, Matthew Macy  wrote:
>> >>
>> >> Warnings find bugs PERIOD. Although most are not useful, I've found
>> > Some warnings indicate bugs, some warnings are just wrong. If you
>> > have a "may be used uninitialized" warning being a false positive, you
>> > may silences the warning by just set it to zero in the declaration and
>> > you silence it. Other compilers might then correctly report an
>> > assignment without affect...
>>
>> I have yet to see a double assignment be flagged as assignment without
>> effect. If it _does_ occur then we have to disable the warning on the
>> compiler that we have less faith in.
>
>
> Coverity does exactly that.

:-(
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Warner Losh 
On Thu, May 24, 2018 at 12:36 AM, Matthew Macy  wrote:

> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>  wrote:
> >> On 24. May 2018, at 06:51, Matthew Macy  wrote:
> >>
> >> Warnings find bugs PERIOD. Although most are not useful, I've found
> > Some warnings indicate bugs, some warnings are just wrong. If you
> > have a "may be used uninitialized" warning being a false positive, you
> > may silences the warning by just set it to zero in the declaration and
> > you silence it. Other compilers might then correctly report an
> > assignment without affect...
>
> I have yet to see a double assignment be flagged as assignment without
> effect. If it _does_ occur then we have to disable the warning on the
> compiler that we have less faith in.
>

Coverity does exactly that.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Wed, May 23, 2018 at 11:42 PM, Michael Tuexen
 wrote:
>> On 24. May 2018, at 08:36, Matthew Macy  wrote:
>>
>> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>>  wrote:
 On 24. May 2018, at 06:51, Matthew Macy  wrote:

 Warnings find bugs PERIOD. Although most are not useful, I've found
>>> Some warnings indicate bugs, some warnings are just wrong. If you
>>> have a "may be used uninitialized" warning being a false positive, you
>>> may silences the warning by just set it to zero in the declaration and
>>> you silence it. Other compilers might then correctly report an
>>> assignment without affect...
>>
>> I have yet to see a double assignment be flagged as assignment without
>> effect. If it _does_ occur then we have to disable the warning on the
>> compiler that we have less faith in.
> Have seen it in the past in a difference project... But you miss my point:
>
> Not all warnings indicate bugs PERIOD. Some warning are just wrong...

Have you read my follow up? _Many_ Many warnings are wrong. Please
respond to that on what the global policy should be. The value of any
one particular instance of a warning does not merit discussion.
-M











, in this case the assignment should be added with
> a comment /* pacify gcc */.
>
> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
> wrote:
> M> > The initialization isn't useful.
> M>
> M> It silences a gcc warning. So yes it is. It's this exchange which is 
> not useful.
> M>
> M> -M
> M>
> M>
> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> > M> Talk to the gcc devs. The warning is useful even if there are 
> false positives.
> M> > M>
> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
>  wrote:
> M> > M> >   Hi,
> M> > M> >
> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> > M> Author: mmacy
> M> > M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> > M> New Revision: 333860
> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M> > M>
> M> > M> > M> Log:
> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
> actually initialized
> M> > M> > M>
> M> > M> > M> Modified:
> M> > M> > M>   head/sys/kern/kern_sendfile.c
> M> > M> > M>
> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> > M> 
> ==
> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 
> 2018(r333859)
> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 
> 2018(r333860)
> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
> sf_io *sfio, o
> M> > M> > M>  }
> M> > M> > M>
> M> > M> > M>  for (int i = 0; i < npages;) {
> M> > M> > M> -int j, a, count, rv;
> M> > M> > M> +int j, a, count, rv __unused;
> M> > M> > M>
> M> > M> > M>  /* Skip valid pages. */
> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
> PAGE_MASK,
> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M> > M>  if (space == 0) {
> M> > M> > M>  sfio = NULL;
> M> > M> > M>  nios = 0;
> M> > M> > M> +npages = 0;
> M> > M> > M>  goto prepend_header;
> M> > M> > M>  }
> M> > M> > M>  hdr_uio = NULL;
> M> > M> >
> M> > M> > This initialization is redundant and a compiler warning if 
> exists is wrong.
> M> > M> >
> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
> use npages.
> M> > M> >
> M> > M> > --
> M> > M> > Gleb Smirnoff
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff

>>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Michael Tuexen 
> On 24. May 2018, at 08:36, Matthew Macy  wrote:
> 
> On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
>  wrote:
>>> On 24. May 2018, at 06:51, Matthew Macy  wrote:
>>> 
>>> Warnings find bugs PERIOD. Although most are not useful, I've found
>> Some warnings indicate bugs, some warnings are just wrong. If you
>> have a "may be used uninitialized" warning being a false positive, you
>> may silences the warning by just set it to zero in the declaration and
>> you silence it. Other compilers might then correctly report an
>> assignment without affect...
> 
> I have yet to see a double assignment be flagged as assignment without
> effect. If it _does_ occur then we have to disable the warning on the
> compiler that we have less faith in.
Have seen it in the past in a difference project... But you miss my point:

Not all warnings indicate bugs PERIOD. Some warning are just wrong...

Best regards
Michael
> -M
> 
> 
>> 
>> Best regards
>> Michael
>>> quite a number of real issues from compiling with gcc8.
>>> 
>>> If you want to _actually_ be helpful fix these:
>>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
>>> 
>>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
>>> 
>>> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
 Let me repeat again. The warning is a false positive, and thus assignment
 isn't useful. I'm not worried about a single instruction, more about
 polluting the code.
 
 If the warning was escalated to build error, and we did carry about
 building with gcc8, in this case the assignment should be added with
 a comment /* pacify gcc */.
 
 On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
 M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
 wrote:
 M> > The initialization isn't useful.
 M>
 M> It silences a gcc warning. So yes it is. It's this exchange which is 
 not useful.
 M>
 M> -M
 M>
 M>
 M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
 M> > M> Talk to the gcc devs. The warning is useful even if there are 
 false positives.
 M> > M>
 M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
  wrote:
 M> > M> >   Hi,
 M> > M> >
 M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
 M> > M> > M> Author: mmacy
 M> > M> > M> Date: Sat May 19 05:10:51 2018
 M> > M> > M> New Revision: 333860
 M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
 M> > M> > M>
 M> > M> > M> Log:
 M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
 actually initialized
 M> > M> > M>
 M> > M> > M> Modified:
 M> > M> > M>   head/sys/kern/kern_sendfile.c
 M> > M> > M>
 M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
 M> > M> > M> 
 ==
 M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018 
(r333859)
 M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018 
(r333860)
 M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
 sf_io *sfio, o
 M> > M> > M>  }
 M> > M> > M>
 M> > M> > M>  for (int i = 0; i < npages;) {
 M> > M> > M> -int j, a, count, rv;
 M> > M> > M> +int j, a, count, rv __unused;
 M> > M> > M>
 M> > M> > M>  /* Skip valid pages. */
 M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
 PAGE_MASK,
 M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
 M> > M> > M>  if (space == 0) {
 M> > M> > M>  sfio = NULL;
 M> > M> > M>  nios = 0;
 M> > M> > M> +npages = 0;
 M> > M> > M>  goto prepend_header;
 M> > M> > M>  }
 M> > M> > M>  hdr_uio = NULL;
 M> > M> >
 M> > M> > This initialization is redundant and a compiler warning if 
 exists is wrong.
 M> > M> >
 M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
 use npages.
 M> > M> >
 M> > M> > --
 M> > M> > Gleb Smirnoff
 M> >
 M> > --
 M> > Gleb Smirnoff
 
 --
 Gleb Smirnoff
>>> 
>> 

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Michael Tuexen 
> On 24. May 2018, at 06:51, Matthew Macy  wrote:
> 
> Warnings find bugs PERIOD. Although most are not useful, I've found
Some warnings indicate bugs, some warnings are just wrong. If you
have a "may be used uninitialized" warning being a false positive, you
may silences the warning by just set it to zero in the declaration and
you silence it. Other compilers might then correctly report an
assignment without affect...

Best regards
Michael
> quite a number of real issues from compiling with gcc8.
> 
> If you want to _actually_ be helpful fix these:
> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
> 
> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
> 
> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
>> Let me repeat again. The warning is a false positive, and thus assignment
>> isn't useful. I'm not worried about a single instruction, more about
>> polluting the code.
>> 
>> If the warning was escalated to build error, and we did carry about
>> building with gcc8, in this case the assignment should be added with
>> a comment /* pacify gcc */.
>> 
>> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
>> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
>> wrote:
>> M> > The initialization isn't useful.
>> M>
>> M> It silences a gcc warning. So yes it is. It's this exchange which is not 
>> useful.
>> M>
>> M> -M
>> M>
>> M>
>> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
>> M> > M> Talk to the gcc devs. The warning is useful even if there are false 
>> positives.
>> M> > M>
>> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  
>> wrote:
>> M> > M> >   Hi,
>> M> > M> >
>> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
>> M> > M> > M> Author: mmacy
>> M> > M> > M> Date: Sat May 19 05:10:51 2018
>> M> > M> > M> New Revision: 333860
>> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
>> M> > M> > M>
>> M> > M> > M> Log:
>> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
>> actually initialized
>> M> > M> > M>
>> M> > M> > M> Modified:
>> M> > M> > M>   head/sys/kern/kern_sendfile.c
>> M> > M> > M>
>> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
>> M> > M> > M> 
>> ==
>> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018   
>>  (r333859)
>> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018   
>>  (r333860)
>> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
>> sf_io *sfio, o
>> M> > M> > M>  }
>> M> > M> > M>
>> M> > M> > M>  for (int i = 0; i < npages;) {
>> M> > M> > M> -int j, a, count, rv;
>> M> > M> > M> +int j, a, count, rv __unused;
>> M> > M> > M>
>> M> > M> > M>  /* Skip valid pages. */
>> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
>> PAGE_MASK,
>> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
>> M> > M> > M>  if (space == 0) {
>> M> > M> > M>  sfio = NULL;
>> M> > M> > M>  nios = 0;
>> M> > M> > M> +npages = 0;
>> M> > M> > M>  goto prepend_header;
>> M> > M> > M>  }
>> M> > M> > M>  hdr_uio = NULL;
>> M> > M> >
>> M> > M> > This initialization is redundant and a compiler warning if exists 
>> is wrong.
>> M> > M> >
>> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
>> use npages.
>> M> > M> >
>> M> > M> > --
>> M> > M> > Gleb Smirnoff
>> M> >
>> M> > --
>> M> > Gleb Smirnoff
>> 
>> --
>> Gleb Smirnoff
> 

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Wed, May 23, 2018 at 10:25 PM, Gleb Smirnoff  wrote:
> On Wed, May 23, 2018 at 10:13:25PM -0700, Matthew Macy wrote:
> M> On Wed, May 23, 2018 at 10:07 PM, Gleb Smirnoff  
> wrote:
> M> > Can you please explain the bug supposed to be fixed by r333860 QUESTION 
> MARK
> M>
> M> Did I say it fixed a bug? Or are you saying we should just turn off
> M> compiler warnings because Gleb Smirnoff knows better?
>
> You just did say "Warnings find bugs PERIOD." This implicitly claimed
> that shutting this one fixed a bug.
>
> Indeed, I am saying that in this particular case Gleb Smirnoff knows
> better than gcc8 does. However, I'm not saying to turn off compiler warnings.
> Never said that. I am saying (several times already) not to insert redundant
> code to shut up a warning that is false.

I'm not saying you're right or wrong. On initial inspection it
appeared uninitialized. I may be wrong. I never asserted that you were
wrong. What I'm trying to understand is what your desired policy with
respect to warnings is. If the author knows that a warning is false he
should

a) Disable that warning for the entire build because it is wrong in one case?

b) Edit the Makefile to disable the warning for that file?

c) Leave the warning enabled and just permit a noisy build where the
warning potentially drowns out real issues?

Right now you're discussing a single line in a single file when in
fact this is a kernel wide policy issue. All three of these choices
seem less desirable than selectively placating the compiler as I have
en masse for much of the tree.


-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Wed, May 23, 2018 at 11:35 PM, Michael Tuexen
 wrote:
>> On 24. May 2018, at 06:51, Matthew Macy  wrote:
>>
>> Warnings find bugs PERIOD. Although most are not useful, I've found
> Some warnings indicate bugs, some warnings are just wrong. If you
> have a "may be used uninitialized" warning being a false positive, you
> may silences the warning by just set it to zero in the declaration and
> you silence it. Other compilers might then correctly report an
> assignment without affect...

I have yet to see a double assignment be flagged as assignment without
effect. If it _does_ occur then we have to disable the warning on the
compiler that we have less faith in.
-M


>
> Best regards
> Michael
>> quite a number of real issues from compiling with gcc8.
>>
>> If you want to _actually_ be helpful fix these:
>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
>>
>> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
>>
>> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
>>> Let me repeat again. The warning is a false positive, and thus assignment
>>> isn't useful. I'm not worried about a single instruction, more about
>>> polluting the code.
>>>
>>> If the warning was escalated to build error, and we did carry about
>>> building with gcc8, in this case the assignment should be added with
>>> a comment /* pacify gcc */.
>>>
>>> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
>>> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
>>> wrote:
>>> M> > The initialization isn't useful.
>>> M>
>>> M> It silences a gcc warning. So yes it is. It's this exchange which is not 
>>> useful.
>>> M>
>>> M> -M
>>> M>
>>> M>
>>> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
>>> M> > M> Talk to the gcc devs. The warning is useful even if there are false 
>>> positives.
>>> M> > M>
>>> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
>>>  wrote:
>>> M> > M> >   Hi,
>>> M> > M> >
>>> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
>>> M> > M> > M> Author: mmacy
>>> M> > M> > M> Date: Sat May 19 05:10:51 2018
>>> M> > M> > M> New Revision: 333860
>>> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
>>> M> > M> > M>
>>> M> > M> > M> Log:
>>> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
>>> actually initialized
>>> M> > M> > M>
>>> M> > M> > M> Modified:
>>> M> > M> > M>   head/sys/kern/kern_sendfile.c
>>> M> > M> > M>
>>> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
>>> M> > M> > M> 
>>> ==
>>> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018  
>>>   (r333859)
>>> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018  
>>>   (r333860)
>>> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
>>> sf_io *sfio, o
>>> M> > M> > M>  }
>>> M> > M> > M>
>>> M> > M> > M>  for (int i = 0; i < npages;) {
>>> M> > M> > M> -int j, a, count, rv;
>>> M> > M> > M> +int j, a, count, rv __unused;
>>> M> > M> > M>
>>> M> > M> > M>  /* Skip valid pages. */
>>> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
>>> PAGE_MASK,
>>> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
>>> M> > M> > M>  if (space == 0) {
>>> M> > M> > M>  sfio = NULL;
>>> M> > M> > M>  nios = 0;
>>> M> > M> > M> +npages = 0;
>>> M> > M> > M>  goto prepend_header;
>>> M> > M> > M>  }
>>> M> > M> > M>  hdr_uio = NULL;
>>> M> > M> >
>>> M> > M> > This initialization is redundant and a compiler warning if exists 
>>> is wrong.
>>> M> > M> >
>>> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
>>> use npages.
>>> M> > M> >
>>> M> > M> > --
>>> M> > M> > Gleb Smirnoff
>>> M> >
>>> M> > --
>>> M> > Gleb Smirnoff
>>>
>>> --
>>> Gleb Smirnoff
>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Gleb Smirnoff 
On Wed, May 23, 2018 at 10:13:25PM -0700, Matthew Macy wrote:
M> On Wed, May 23, 2018 at 10:07 PM, Gleb Smirnoff  wrote:
M> > Can you please explain the bug supposed to be fixed by r333860 QUESTION 
MARK
M> 
M> Did I say it fixed a bug? Or are you saying we should just turn off
M> compiler warnings because Gleb Smirnoff knows better?

You just did say "Warnings find bugs PERIOD." This implicitly claimed
that shutting this one fixed a bug.

Indeed, I am saying that in this particular case Gleb Smirnoff knows
better than gcc8 does. However, I'm not saying to turn off compiler warnings.
Never said that. I am saying (several times already) not to insert redundant
code to shut up a warning that is false.

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Gleb Smirnoff 
Can you please explain the bug supposed to be fixed by r333860 QUESTION MARK

On Wed, May 23, 2018 at 09:51:34PM -0700, Matthew Macy wrote:
M> Warnings find bugs PERIOD. Although most are not useful, I've found
M> quite a number of real issues from compiling with gcc8.
M> 
M> If you want to _actually_ be helpful fix these:
M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
M> 
M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
M> 
M> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
M> > Let me repeat again. The warning is a false positive, and thus assignment
M> > isn't useful. I'm not worried about a single instruction, more about
M> > polluting the code.
M> >
M> > If the warning was escalated to build error, and we did carry about
M> > building with gcc8, in this case the assignment should be added with
M> > a comment /* pacify gcc */.
M> >
M> > On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
M> > M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
wrote:
M> > M> > The initialization isn't useful.
M> > M>
M> > M> It silences a gcc warning. So yes it is. It's this exchange which is 
not useful.
M> > M>
M> > M> -M
M> > M>
M> > M>
M> > M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
M> > M> > M> Talk to the gcc devs. The warning is useful even if there are 
false positives.
M> > M> > M>
M> > M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
 wrote:
M> > M> > M> >   Hi,
M> > M> > M> >
M> > M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
M> > M> > M> > M> Author: mmacy
M> > M> > M> > M> Date: Sat May 19 05:10:51 2018
M> > M> > M> > M> New Revision: 333860
M> > M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
M> > M> > M> > M>
M> > M> > M> > M> Log:
M> > M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
actually initialized
M> > M> > M> > M>
M> > M> > M> > M> Modified:
M> > M> > M> > M>   head/sys/kern/kern_sendfile.c
M> > M> > M> > M>
M> > M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
M> > M> > M> > M> 
==
M> > M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018 
   (r333859)
M> > M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018 
   (r333860)
M> > M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
sf_io *sfio, o
M> > M> > M> > M>  }
M> > M> > M> > M>
M> > M> > M> > M>  for (int i = 0; i < npages;) {
M> > M> > M> > M> -int j, a, count, rv;
M> > M> > M> > M> +int j, a, count, rv __unused;
M> > M> > M> > M>
M> > M> > M> > M>  /* Skip valid pages. */
M> > M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
PAGE_MASK,
M> > M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
M> > M> > M> > M>  if (space == 0) {
M> > M> > M> > M>  sfio = NULL;
M> > M> > M> > M>  nios = 0;
M> > M> > M> > M> +npages = 0;
M> > M> > M> > M>  goto prepend_header;
M> > M> > M> > M>  }
M> > M> > M> > M>  hdr_uio = NULL;
M> > M> > M> >
M> > M> > M> > This initialization is redundant and a compiler warning if 
exists is wrong.
M> > M> > M> >
M> > M> > M> > If we jump down to prepend_header with nios == 0, we won't ever 
use npages.
M> > M> > M> >
M> > M> > M> > --
M> > M> > M> > Gleb Smirnoff
M> > M> >
M> > M> > --
M> > M> > Gleb Smirnoff
M> >
M> > --
M> > Gleb Smirnoff

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
Warnings find bugs PERIOD. Although most are not useful, I've found
quite a number of real issues from compiling with gcc8.

If you want to _actually_ be helpful fix these:
https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log

https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log

On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
> Let me repeat again. The warning is a false positive, and thus assignment
> isn't useful. I'm not worried about a single instruction, more about
> polluting the code.
>
> If the warning was escalated to build error, and we did carry about
> building with gcc8, in this case the assignment should be added with
> a comment /* pacify gcc */.
>
> On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  wrote:
> M> > The initialization isn't useful.
> M>
> M> It silences a gcc warning. So yes it is. It's this exchange which is not 
> useful.
> M>
> M> -M
> M>
> M>
> M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> > M> Talk to the gcc devs. The warning is useful even if there are false 
> positives.
> M> > M>
> M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  
> wrote:
> M> > M> >   Hi,
> M> > M> >
> M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> > M> Author: mmacy
> M> > M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> > M> New Revision: 333860
> M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M> > M>
> M> > M> > M> Log:
> M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
> actually initialized
> M> > M> > M>
> M> > M> > M> Modified:
> M> > M> > M>   head/sys/kern/kern_sendfile.c
> M> > M> > M>
> M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> > M> 
> ==
> M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
> sf_io *sfio, o
> M> > M> > M>  }
> M> > M> > M>
> M> > M> > M>  for (int i = 0; i < npages;) {
> M> > M> > M> -int j, a, count, rv;
> M> > M> > M> +int j, a, count, rv __unused;
> M> > M> > M>
> M> > M> > M>  /* Skip valid pages. */
> M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
> PAGE_MASK,
> M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M> > M>  if (space == 0) {
> M> > M> > M>  sfio = NULL;
> M> > M> > M>  nios = 0;
> M> > M> > M> +npages = 0;
> M> > M> > M>  goto prepend_header;
> M> > M> > M>  }
> M> > M> > M>  hdr_uio = NULL;
> M> > M> >
> M> > M> > This initialization is redundant and a compiler warning if exists 
> is wrong.
> M> > M> >
> M> > M> > If we jump down to prepend_header with nios == 0, we won't ever use 
> npages.
> M> > M> >
> M> > M> > --
> M> > M> > Gleb Smirnoff
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Gleb Smirnoff 
Let me repeat again. The warning is a false positive, and thus assignment
isn't useful. I'm not worried about a single instruction, more about
polluting the code.

If the warning was escalated to build error, and we did carry about
building with gcc8, in this case the assignment should be added with
a comment /* pacify gcc */.

On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  wrote:
M> > The initialization isn't useful.
M> 
M> It silences a gcc warning. So yes it is. It's this exchange which is not 
useful.
M> 
M> -M
M> 
M> 
M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
M> > M> Talk to the gcc devs. The warning is useful even if there are false 
positives.
M> > M>
M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  
wrote:
M> > M> >   Hi,
M> > M> >
M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
M> > M> > M> Author: mmacy
M> > M> > M> Date: Sat May 19 05:10:51 2018
M> > M> > M> New Revision: 333860
M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
M> > M> > M>
M> > M> > M> Log:
M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
actually initialized
M> > M> > M>
M> > M> > M> Modified:
M> > M> > M>   head/sys/kern/kern_sendfile.c
M> > M> > M>
M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
M> > M> > M> 
==
M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018  
  (r333859)
M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018  
  (r333860)
M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io 
*sfio, o
M> > M> > M>  }
M> > M> > M>
M> > M> > M>  for (int i = 0; i < npages;) {
M> > M> > M> -int j, a, count, rv;
M> > M> > M> +int j, a, count, rv __unused;
M> > M> > M>
M> > M> > M>  /* Skip valid pages. */
M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
M> > M> > M>  if (space == 0) {
M> > M> > M>  sfio = NULL;
M> > M> > M>  nios = 0;
M> > M> > M> +npages = 0;
M> > M> > M>  goto prepend_header;
M> > M> > M>  }
M> > M> > M>  hdr_uio = NULL;
M> > M> >
M> > M> > This initialization is redundant and a compiler warning if exists is 
wrong.
M> > M> >
M> > M> > If we jump down to prepend_header with nios == 0, we won't ever use 
npages.
M> > M> >
M> > M> > --
M> > M> > Gleb Smirnoff
M> >
M> > --
M> > Gleb Smirnoff

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-24 Thread Matthew Macy 
On Wed, May 23, 2018 at 10:07 PM, Gleb Smirnoff  wrote:
> Can you please explain the bug supposed to be fixed by r333860 QUESTION MARK

Did I say it fixed a bug? Or are you saying we should just turn off
compiler warnings because Gleb Smirnoff knows better?

-M


> On Wed, May 23, 2018 at 09:51:34PM -0700, Matthew Macy wrote:
> M> Warnings find bugs PERIOD. Although most are not useful, I've found
> M> quite a number of real issues from compiling with gcc8.
> M>
> M> If you want to _actually_ be helpful fix these:
> M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC-NODEBUG.log
> M>
> M> https://people.freebsd.org/~mmacy/gcc8logs/GENERIC.log
> M>
> M> On Wed, May 23, 2018 at 9:42 PM, Gleb Smirnoff  wrote:
> M> > Let me repeat again. The warning is a false positive, and thus assignment
> M> > isn't useful. I'm not worried about a single instruction, more about
> M> > polluting the code.
> M> >
> M> > If the warning was escalated to build error, and we did carry about
> M> > building with gcc8, in this case the assignment should be added with
> M> > a comment /* pacify gcc */.
> M> >
> M> > On Wed, May 23, 2018 at 03:59:33PM -0700, Matthew Macy wrote:
> M> > M> On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  
> wrote:
> M> > M> > The initialization isn't useful.
> M> > M>
> M> > M> It silences a gcc warning. So yes it is. It's this exchange which is 
> not useful.
> M> > M>
> M> > M> -M
> M> > M>
> M> > M>
> M> > M> > On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> > M> > M> Talk to the gcc devs. The warning is useful even if there are 
> false positives.
> M> > M> > M>
> M> > M> > M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff 
>  wrote:
> M> > M> > M> >   Hi,
> M> > M> > M> >
> M> > M> > M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> > M> > M> Author: mmacy
> M> > M> > M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> > M> > M> New Revision: 333860
> M> > M> > M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M> > M> > M>
> M> > M> > M> > M> Log:
> M> > M> > M> > M>   sendfile: annotate unused value and ensure that npages is 
> actually initialized
> M> > M> > M> > M>
> M> > M> > M> > M> Modified:
> M> > M> > M> > M>   head/sys/kern/kern_sendfile.c
> M> > M> > M> > M>
> M> > M> > M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> > M> > M> 
> ==
> M> > M> > M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 
> 2018(r333859)
> M> > M> > M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 
> 2018(r333860)
> M> > M> > M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct 
> sf_io *sfio, o
> M> > M> > M> > M>  }
> M> > M> > M> > M>
> M> > M> > M> > M>  for (int i = 0; i < npages;) {
> M> > M> > M> > M> -int j, a, count, rv;
> M> > M> > M> > M> +int j, a, count, rv __unused;
> M> > M> > M> > M>
> M> > M> > M> > M>  /* Skip valid pages. */
> M> > M> > M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & 
> PAGE_MASK,
> M> > M> > M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M> > M> > M>  if (space == 0) {
> M> > M> > M> > M>  sfio = NULL;
> M> > M> > M> > M>  nios = 0;
> M> > M> > M> > M> +npages = 0;
> M> > M> > M> > M>  goto prepend_header;
> M> > M> > M> > M>  }
> M> > M> > M> > M>  hdr_uio = NULL;
> M> > M> > M> >
> M> > M> > M> > This initialization is redundant and a compiler warning if 
> exists is wrong.
> M> > M> > M> >
> M> > M> > M> > If we jump down to prepend_header with nios == 0, we won't 
> ever use npages.
> M> > M> > M> >
> M> > M> > M> > --
> M> > M> > M> > Gleb Smirnoff
> M> > M> >
> M> > M> > --
> M> > M> > Gleb Smirnoff
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-23 Thread Matthew Macy 
On Wed, May 23, 2018 at 3:57 PM, Gleb Smirnoff  wrote:
> The initialization isn't useful.

It silences a gcc warning. So yes it is. It's this exchange which is not useful.

-M


> On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
> M> Talk to the gcc devs. The warning is useful even if there are false 
> positives.
> M>
> M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  wrote:
> M> >   Hi,
> M> >
> M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> > M> Author: mmacy
> M> > M> Date: Sat May 19 05:10:51 2018
> M> > M> New Revision: 333860
> M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M> > M>
> M> > M> Log:
> M> > M>   sendfile: annotate unused value and ensure that npages is actually 
> initialized
> M> > M>
> M> > M> Modified:
> M> > M>   head/sys/kern/kern_sendfile.c
> M> > M>
> M> > M> Modified: head/sys/kern/kern_sendfile.c
> M> > M> 
> ==
> M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io 
> *sfio, o
> M> > M>  }
> M> > M>
> M> > M>  for (int i = 0; i < npages;) {
> M> > M> -int j, a, count, rv;
> M> > M> +int j, a, count, rv __unused;
> M> > M>
> M> > M>  /* Skip valid pages. */
> M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
> M> > M> @@ -688,6 +688,7 @@ retry_space:
> M> > M>  if (space == 0) {
> M> > M>  sfio = NULL;
> M> > M>  nios = 0;
> M> > M> +npages = 0;
> M> > M>  goto prepend_header;
> M> > M>  }
> M> > M>  hdr_uio = NULL;
> M> >
> M> > This initialization is redundant and a compiler warning if exists is 
> wrong.
> M> >
> M> > If we jump down to prepend_header with nios == 0, we won't ever use 
> npages.
> M> >
> M> > --
> M> > Gleb Smirnoff
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-23 Thread Gleb Smirnoff 
The initialization isn't useful.

On Wed, May 23, 2018 at 03:52:42PM -0700, Matthew Macy wrote:
M> Talk to the gcc devs. The warning is useful even if there are false 
positives.
M> 
M> On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  wrote:
M> >   Hi,
M> >
M> > On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
M> > M> Author: mmacy
M> > M> Date: Sat May 19 05:10:51 2018
M> > M> New Revision: 333860
M> > M> URL: https://svnweb.freebsd.org/changeset/base/333860
M> > M>
M> > M> Log:
M> > M>   sendfile: annotate unused value and ensure that npages is actually 
initialized
M> > M>
M> > M> Modified:
M> > M>   head/sys/kern/kern_sendfile.c
M> > M>
M> > M> Modified: head/sys/kern/kern_sendfile.c
M> > M> 
==
M> > M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
(r333859)
M> > M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
(r333860)
M> > M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io 
*sfio, o
M> > M>  }
M> > M>
M> > M>  for (int i = 0; i < npages;) {
M> > M> -int j, a, count, rv;
M> > M> +int j, a, count, rv __unused;
M> > M>
M> > M>  /* Skip valid pages. */
M> > M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
M> > M> @@ -688,6 +688,7 @@ retry_space:
M> > M>  if (space == 0) {
M> > M>  sfio = NULL;
M> > M>  nios = 0;
M> > M> +npages = 0;
M> > M>  goto prepend_header;
M> > M>  }
M> > M>  hdr_uio = NULL;
M> >
M> > This initialization is redundant and a compiler warning if exists is wrong.
M> >
M> > If we jump down to prepend_header with nios == 0, we won't ever use npages.
M> >
M> > --
M> > Gleb Smirnoff

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-23 Thread Matthew Macy 
Talk to the gcc devs. The warning is useful even if there are false positives.

On Wed, May 23, 2018 at 3:27 PM, Gleb Smirnoff  wrote:
>   Hi,
>
> On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
> M> Author: mmacy
> M> Date: Sat May 19 05:10:51 2018
> M> New Revision: 333860
> M> URL: https://svnweb.freebsd.org/changeset/base/333860
> M>
> M> Log:
> M>   sendfile: annotate unused value and ensure that npages is actually 
> initialized
> M>
> M> Modified:
> M>   head/sys/kern/kern_sendfile.c
> M>
> M> Modified: head/sys/kern/kern_sendfile.c
> M> 
> ==
> M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
> (r333859)
> M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
> (r333860)
> M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, o
> M>  }
> M>
> M>  for (int i = 0; i < npages;) {
> M> -int j, a, count, rv;
> M> +int j, a, count, rv __unused;
> M>
> M>  /* Skip valid pages. */
> M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
> M> @@ -688,6 +688,7 @@ retry_space:
> M>  if (space == 0) {
> M>  sfio = NULL;
> M>  nios = 0;
> M> +npages = 0;
> M>  goto prepend_header;
> M>  }
> M>  hdr_uio = NULL;
>
> This initialization is redundant and a compiler warning if exists is wrong.
>
> If we jump down to prepend_header with nios == 0, we won't ever use npages.
>
> --
> Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r333860 - head/sys/kern

2018-05-23 Thread Gleb Smirnoff 
  Hi,

On Sat, May 19, 2018 at 05:10:52AM +, Matt Macy wrote:
M> Author: mmacy
M> Date: Sat May 19 05:10:51 2018
M> New Revision: 333860
M> URL: https://svnweb.freebsd.org/changeset/base/333860
M> 
M> Log:
M>   sendfile: annotate unused value and ensure that npages is actually 
initialized
M> 
M> Modified:
M>   head/sys/kern/kern_sendfile.c
M> 
M> Modified: head/sys/kern/kern_sendfile.c
M> 
==
M> --- head/sys/kern/kern_sendfile.cSat May 19 05:09:10 2018
(r333859)
M> +++ head/sys/kern/kern_sendfile.cSat May 19 05:10:51 2018
(r333860)
M> @@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, o
M>  }
M>  
M>  for (int i = 0; i < npages;) {
M> -int j, a, count, rv;
M> +int j, a, count, rv __unused;
M>  
M>  /* Skip valid pages. */
M>  if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
M> @@ -688,6 +688,7 @@ retry_space:
M>  if (space == 0) {
M>  sfio = NULL;
M>  nios = 0;
M> +npages = 0;
M>  goto prepend_header;
M>  }
M>  hdr_uio = NULL;

This initialization is redundant and a compiler warning if exists is wrong.

If we jump down to prepend_header with nios == 0, we won't ever use npages.

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r333860 - head/sys/kern

2018-05-18 Thread Matt Macy 
Author: mmacy
Date: Sat May 19 05:10:51 2018
New Revision: 333860
URL: https://svnweb.freebsd.org/changeset/base/333860

Log:
  sendfile: annotate unused value and ensure that npages is actually initialized

Modified:
  head/sys/kern/kern_sendfile.c

Modified: head/sys/kern/kern_sendfile.c
==
--- head/sys/kern/kern_sendfile.c   Sat May 19 05:09:10 2018
(r333859)
+++ head/sys/kern/kern_sendfile.c   Sat May 19 05:10:51 2018
(r333860)
@@ -341,7 +341,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, o
}
 
for (int i = 0; i < npages;) {
-   int j, a, count, rv;
+   int j, a, count, rv __unused;
 
/* Skip valid pages. */
if (vm_page_is_valid(pa[i], vmoff(i, off) & PAGE_MASK,
@@ -688,6 +688,7 @@ retry_space:
if (space == 0) {
sfio = NULL;
nios = 0;
+   npages = 0;
goto prepend_header;
}
hdr_uio = NULL;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"