Re: Build Warnings: spotting them, and cleaning them up

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 16, 2016 at 2:05 PM, Gregory Farnum <gfarnum@xxxxxxxxxx> wrote:
> Unfortunately, we've got a problem: boost is notorious for build
> warnings. At the moment, I'm only seeing one (repeated many times
> over) resulting from use of the "remove_whitespace" iterator
> functionality in RGW. Does anybody know a good way to handle that?

Oh, there's also a warning coming from cls_rbd's use of the
optional_io.hpp boost header. I'm not clear on if it's actually an
internal warning or not ("warning: 't' may be used uninitialized in
this function [-Wmaybe-uninitialized]") since "t" is declared in our
include/encoding.h.

>
> We also have another (seemingly real) warning: the BlueStore
> BitAllocator says there are missing return statements in
> BmapEntry::operator new [](size_t). I didn't look at the code; maybe
> it's failing to detect return statements in an if-else block. But that
> should be fixed up as well.

And yet another signed/unsigned comparison warning popped up in PG.cc
from "MAX(min, cct->_conf->osd_scrub_chunk_max),"


On Wed, Nov 16, 2016 at 2:10 PM, Adam C. Emerson <aemerson@xxxxxxxxxx> wrote:
> On 16/11/2016, Gregory Farnum wrote:
>> It turns out, Jenkins assumes the build is good if the build script
>> returns 0 — it doesn't know anything about the actual text output and
>> doesn't scan for warnings.
>> I'm not sure how hard it would be to change things from that end. From
>> the *compilation* end, Casey suggests the "nicest" thing would be to
>> set the -Werr flag, which turns all warnings into errors. Since we
>> like warning-free code, that's definitely appealing to me.
>> Unfortunately, we've got a problem: boost is notorious for build
>> warnings. At the moment, I'm only seeing one (repeated many times
>> over) resulting from use of the "remove_whitespace" iterator
>> functionality in RGW. Does anybody know a good way to handle that?
>
> I have a fix for this in my Clangtastic branch, it turns off warnings
> in the building of boost and makes inluded boost a system include.

Is that branch going to merge imminently? If not maybe we could pull
out this and the BlueStore fixes for fast merging. :)

>
>> We also have another (seemingly real) warning: the BlueStore
>> BitAllocator says there are missing return statements in
>> BmapEntry::operator new [](size_t). I didn't look at the code; maybe
>> it's failing to detect return statements in an if-else block. But that
>> should be fixed up as well.
>
> I have a fix for that I'm about to push, I just have it return nullptr
> and mark it noexcept.
>
> (We could also make it throw something and remove the assert entirely,
> but this is more in tune with the current pattern.)
>
>> 1) The Ceph source should build warning-free. If we have warnings
>> from external code we still trust, we need a way to suppress *only*
>> those warnings so that issues we introduce can easily be detected.
>
> This is in principle fairly easily done with cmake hackery. Either
> marking something as a system include path or fiddling the build flags
> per-target.

Is this different from the boost changes you mentioned you already have? :)
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux