On 04/06/2017 20:28, Sage Weil wrote: > On Sun, 4 Jun 2017, Willem Jan Withagen wrote: >> On 04/06/2017 05:28, kefu chai wrote: >>> On Sat, Jun 3, 2017 at 2:50 AM, Willem Jan Withagen <wjw@xxxxxxxxxxx> wrote: >>>> Hi, >>>> >>>> Now that we have -Wvla set: >>>> === >>>> Adds C++ warning flag for C Variable-Length Arrays. >>>> >>>> C VLAs are not supported in C++. However, the GNU compiler allows >>>> them as an extension, which it does not warn about when not in >>>> pedantic mode. Unfortunately, it's easy to accidentally write >>>> a VLA, even unintentionally-- adding this warning will help us >>>> catch that. >>>> === >>>> >>>> Are there going to be attempts to eradicate this usage? >>>> Note that Clang also supports VLAs: >>>> === >>>> GCC and C99 allow an array's size to be determined at run time. This >>>> extension is not permitted in standard C++. However, Clang supports such >>>> variable length arrays for compatibility with GNU C and C99 programs. >>>> === >>>> >>> >>> it's not in C++11 standard. but it's a handy feature in practice. i >>> still have trouble understanding we want to have -Wvla. i was trying >>> to silence this warning option last night but failed to move further >>> after several attempts. see https://github.com/ceph/ceph/pull/15441. >>> >>> probably, Jesse have more insights on this. >> >> It starts with old discussions from before 2009, and it would be hard >> for compiler makers to actually build it. >> >> Both GCC and Clang support it. >> >> And next the major complaint I find about it, is that these structures >> are build on the stack, which might cause stackoverlows (and that mostly >> on embedded computers) That would be something for programmers to be >> aware of. > > I'd consider this a feature more than a risk. It means we can > allocate a modest-sized array without hitting the allocator. My vote > is to just revert -Wvla... > >> I would probably think there are beter things to do, than to chase after >> this. And IMHO the order should have been: Fix the VLAs and only then >> turn the warning on on the master repo. As to not have every body start >> looking at these warning that have just started to pop up. But perhaps >> that is just me, because each and every warning is again a case where I >> need to look for possible incompatabilities. > > Yep, my bad for assuming the cleanup was already done (or trivial)! Took the liberty to submit a revert: https://github.com/ceph/ceph/pull/15469 --WjW -- 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
