search

gearman / gearmand

http://gearman.org/
Other
741 stars 138 forks source link

Issue 278: Fix crashing in libtest/http.cc when '-Wp,-D_GLIBCXX_ASSERTIONS' is given #281

Closed esabol closed 4 years ago

esabol commented 4 years ago

This PR fixes the problems in libtest/http.cc when you compile with CXXFLAGS=-'-Wp,-D_GLIBCXX_ASSERTIONS' on Fedora 31. Refer to issue #278 (and issue #275).

esabol commented 4 years ago

Please restart the following Travis CI jobs: https://travis-ci.org/gearman/gearmand/jobs/653265367 https://travis-ci.org/gearman/gearmand/jobs/653265370 I believe the test failures with these are spurious. Thanks!

cheese commented 4 years ago

LGTM

esabol commented 4 years ago

It would be good to replace &_body[0] by _body.data() at least in both if (HAVE_LIBCURL && _body.size()) blocks.

Why would that be better? I don't see it.

p-alik commented 4 years ago

c++ type should be treated with well-defined type methods https://github.com/gearman/gearmand/blob/a6bd168ca73bd29d0bf13ad946034957f9c86546/libtest/vchar.hpp#L50

In my opinion mixing pure c and c++ style is inappropriate.

esabol commented 4 years ago

In my opinion mixing pure c and c++ style is inappropriate.

Well, that seems to be a separate issue then that should go in a separate PR.

esabol commented 4 years ago

Would it be OK if I merged this?

p-alik commented 4 years ago

I've no objections

SpamapS commented 4 years ago

GMTA @p-alik :)