Adar Dembo has posted comments on this change.

Change subject: KUDU-1325: more crash avoidance during remote bootstrap and 
tablet deletion
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2265/3/src/kudu/consensus/log.h
File src/kudu/consensus/log.h:

Line 156:   // this function cannot return new reader references, but existing 
reader
> can you be more specific here? "will return NULL" or somesuch?
Done


Line 351:   std::shared_ptr<LogReader> reader_;
> worth saying 'NULL after the log is closed'
Done


http://gerrit.cloudera.org:8080/#/c/2265/3/src/kudu/util/make_shared.h
File src/kudu/util/make_shared.h:

Line 38:   // but forward defined as a struct (ext/alloc_traits.h). Clang 
complains
> nit: forward declared
Done


Line 39:   // about this when -Wmismatched-tags is set, which gcc doesn't 
support
> if gcc doesn't support it, could you use #pragma clang instead, and so gcc 
I tried that, but the problem is that gcc warns on #pragma clang too; it 
doesn't ignore it. Here's an example:

  adar@adar-ThinkPad-T540p:/tmp$ cat test.cc
  #include <iostream>
  #include <memory>

  namespace test {
  class Bar {
   public:
    static void create_bar(std::shared_ptr<Bar>* out) {
      *out = std::make_shared<Bar>();
    }

    std::string ToString() {
      return "hello world!";
    }

   private:
  #pragma clang diagnostic push
  #pragma clang diagnostic ignored "-Wmismatched-tags"
    friend class __gnu_cxx::new_allocator<Bar>;
  #pragma clang diagnostic pop

    Bar() {}
  };

  } // namespace test

  using test::Bar;

  int main(int argc, char* argv[]) {
    std::shared_ptr<Bar> b;
    Bar::create_bar(&b);
    std::cout << b->ToString() << std::endl;
    return 0;
  }

  adar@adar-ThinkPad-T540p:/tmp$ clang++ -Wall -std=c++11 -o test test.cc
  adar@adar-ThinkPad-T540p:/tmp$ g++ -Wall -std=c++11 -o test test.cc
  test.cc:19:0: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas]
   #pragma clang diagnostic push
   ^
  test.cc:20:0: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas]
   #pragma clang diagnostic ignored "-Wmismatched-tags"
   ^
  test.cc:22:0: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas]
   #pragma clang diagnostic pop
   ^


-- 
To view, visit http://gerrit.cloudera.org:8080/2265
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6a01c9e6886bd8bf0e286fc3980babc512056286
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to