include/enc: make clang happy

[tchaikov]

No description provided.

[tchaikov]

@wjwithagen and @liewegas i am including #11605 in this PR, and this PR also addresses the build failure in src/test/encoding.cc.

it compiles fine with clang-4.0 on my debian box.

also, i changed #11605 so we are using traits::supported != 0 instead of (bool)traits::supported.

[tchaikov]

ceph/src/include/denc.h:857:10: warning: binding dereferenced null pointer to reference has undefined behavior [-Wnull-dereference]
    denc(*(const T*)nullptr, elem_size);
         ^~~~~~~~~~~~~~~~~~
/var/ceph/ceph/src/include/denc.h:1124:11: note: in instantiation of function template specialization 'denc_traits<std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >, void>::bound_encode<snapid_t>' requested here
  traits::bound_encode(o, len);
          ^
/var/ceph/ceph/src/osd/osd_types.h:2698:7: note: in instantiation of function template specialization 'encode<std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >, denc_traits<std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >, void> >' requested here
    ::encode(old_snaps, bl);
      ^

the warning above from clang is annoying, but no idea what to do with it atm...

[wjwithagen]

@tchaikov
If you are sure that it does not hurt, why not use -Wno-null-dereference for the compilation of this file.

[wjwithagen]

@tchaikov
Any particular reason as to why you prefer var != 0 over (bool)?
Because supported is actually an int?

[tchaikov]

If you are sure that it does not hurt, why not use -Wno-null-dereference for the compilation of this file.

this file is included by many compilation units. so it would be less fun to enumerate all of them and disable this warning when compiling them, and this warning is helpful if enabled under most circumstances. probably we can disable it using pragma directive.

@wjwithagen just want to avoid the c style cast

[wjwithagen]

@tchaikov
Both make sense.
I usually do not really like the #pragma stuff in source files since it really hides the problem very deep in the dungeons to the code. Especially if it needs to propagate into more of the code...

[wjwithagen]

@tchaikov
LGTM for the compiler trouble.
Other than the nullptr stuff, is Clang happy atm.

As you know I'm not the expert on C++, but the if (const T*)nullptr) is sort of NULL in C-speak, what is the purpose of another * to dereference that. It was pointing nowhere

Running the unittest is a different cookie, it still core-dumps, but if this is oke under Linux I suggest to commit this, and then we figure out why the test is failing. That way the FreeBSD jenkins builder does not abort in compilation.

Running main() from gmock_main.cc
[==========] Running 11 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 11 tests from denc
[ RUN      ] denc.denc_counter
[       OK ] denc.denc_counter (0 ms)
[ RUN      ] denc.simple
[       OK ] denc.simple (1 ms)
[ RUN      ] denc.string
[       OK ] denc.string (0 ms)
[ RUN      ] denc.vector
vector<string>
vector<int32_t>
vector<legacy_t>
[       OK ] denc.vector (2 ms)
[ RUN      ] denc.list
list<string>
list<int32_t>
list<legacy_t>
[       OK ] denc.list (2 ms)
[ RUN      ] denc.set
set<string>
set<int32_t>
set<legacy_t>
[       OK ] denc.set (1 ms)
[ RUN      ] denc.foo
00000000  01 01 0c 00 00 00 00 00  00 00 7b 00 00 00 00 00  |..........{.....|
00000010  00 00                                             |..|
00000012
[       OK ] denc.foo (0 ms)
[ RUN      ] denc.bar
[       OK ] denc.bar (1 ms)
[ RUN      ] denc.pair
[       OK ] denc.pair (0 ms)
[ RUN      ] denc.map
map<string,foo_t>
Segmentation fault (core dumped)

[tchaikov]

@wjwithagen strange enough, the test passes on linux.

[wjwithagen]

@tchaikov
I don't say that very easy, since allmost always it is pilot error.
But I could be that I'm running into a compiler bug.

Backtrace

Program received signal SIGSEGV, Segmentation fault.
0x00000000005bc6dc in ceph::buffer::ptr::c_str (this=0x7fffffffd158) at /usr/srcs/Ceph/work/ceph/src/common/buffer.cc:901
901         return _raw->get_data() + _off;
(gdb) p _raw
$10 = (ceph::buffer::raw *) 0x80784f048
(gdb) bt
#0  0x00000000005bc6dc in ceph::buffer::ptr::c_str (this=0x7fffffffd158) at /usr/srcs/Ceph/work/ceph/src/common/buffer.cc:901
#1  0x00000000004bc457 in ceph::buffer::list::contiguous_appender::~contiguous_appender (this=0x7fffffffd148)
    at /usr/srcs/Ceph/work/ceph/src/include/buffer.h:514
#2  0x00000000004bce02 in test_denc<std::__1::map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, foo_t, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, foo_t> > > > (v=...) at /usr/srcs/Ceph/work/ceph/src/test/test_denc.cc:50
#3  0x00000000004acdee in denc_map_Test::TestBody (this=0x807837010) at /usr/srcs/Ceph/work/ceph/src/test/test_denc.cc:413

    public:
      ~contiguous_appender() {
        if (bp.have_raw()) {
          // we allocated a new buffer
          bp.set_length(pos - bp.c_str());
          pbl->append(std::move(bp));
        } else {

And c_str()

  char *buffer::ptr::c_str() {
    assert(_raw);
    if (buffer_track_c_str)
      buffer_c_str_accesses.inc();
    return _raw->get_data() + _off;
  }

Get data is even more simple:

    virtual char *get_data() {
      return data;
    }

So I'm sort of wondering why this would SIGSEGV if _raw is pointing in valid space....

 p *_raw
$12 = {_vptr$raw = 0x0, data = 0x80784f000 "\003", len = 72, nref = {val = 1}, crc_spinlock = 0, crc_map = {__tree_ = {
      __begin_node_ = 0x80784f078,
      __pair1_ = {<std::__1::__libcpp_compressed_pair_imp<std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>, std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<std::__1::pair<unsigned long, unsigned long>, std::__1::pair<unsigned int, unsigned int> >, void*> >, 2>> = {<std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<std::__1::pair<unsigned long, unsigned long>, std::__1::pair<unsigned int, unsigned int> >, void*> >> = {<No data fields>}, __first_ = {
            __left_ = 0x0}}, <No data fields>},
      __pair3_ = {<std::__1::__libcpp_compressed_pair_imp<unsigned long, std::__1::__map_value_compare<std::__1::pair<unsigned long, unsigned long>, std::__1::__value_type<std::__1::pair<unsigned long, unsigned long>, std::__1::pair<unsigned int, unsigned int> >, std::__1::less<std::__1::pair<unsigned long, unsigned long> >, true>, 2>> = {<std::__1::__map_value_compare<std::__1::pair<unsigned long, unsigned long>, std::__1::__value_type<std::__1::pair<unsigned long, unsigned long>, std::__1::pair<unsigned int, unsigned int> >, std::__1::less<std::__1::pair<unsigned long, unsigned long> >, true>> = {<std::__1::less<std::__1::pair<unsigned long, unsigned long> >> = {<std::__1::binary_function<std::__1::pair<unsigned long, unsigned long>, std::__1::pair<unsigned long, unsigned long>, bool>> = {<No data fields>}, <No data fields>}, <No data fields>}, __first_ = 0}, <No data fields>}}}}

(gdb) p _raw.data
$13 = 0x80784f000 "\003"

So gdb seems to be able to do this.
And this faults with both 3.8.0 and 4.0-devel

[tchaikov]

@liewegas could you take a look?

[liewegas]

lgtm!

[liewegas]

Oh, didn't see @wjwithagen's comment. Probably this is the problem: "_vptr$raw = 0x0"? I'm stumped. I'd starting adding cout lines to see if c_str() is callable earlier, etc..