Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix somes warning when compiling with Visual Studio 2019 on x64 target #8125

Merged
merged 1 commit into from Dec 30, 2020
Merged

Fix somes warning when compiling with Visual Studio 2019 on x64 target #8125

merged 1 commit into from Dec 30, 2020

Conversation

gvollant
Copy link
Contributor

@gvollant gvollant commented Dec 6, 2020

In visual studio 2019 x64 target, pointer size and size_t are 64 bits and int are 32 bits
This commit uses size_t for buffer size

@google-cla google-cla bot added the cla: yes label Dec 6, 2020
@gvollant
Copy link
Contributor Author

@gvollant gvollant commented Dec 15, 2020

These warning fixes are simple.
I don't understand why it's no "mergeable"

gillesvollant@gv-w81x64:/mnt/w$ diff -c /mnt/n/amalib_external_lib_cache/protobuf-cpp-3.14.0_static_bld_gcc_ltofull/src/google/protobuf/parse_context.h /mnt/y/protobu
f/src/google/protobuf/parse_context.h
*** /mnt/n/amalib_external_lib_cache/protobuf-cpp-3.14.0_static_bld_gcc_ltofull/src/google/protobuf/parse_context.h     2020-11-13 23:55:22.000000000 +0100
--- /mnt/y/protobuf/src/google/protobuf/parse_context.h 2020-12-15 14:33:50.510179200 +0100
***************
*** 208,214 ****
    bool DoneWithCheck(const char** ptr, int d) {
      GOOGLE_DCHECK(*ptr);
      if (PROTOBUF_PREDICT_TRUE(*ptr < limit_end_)) return false;
!     int overrun = *ptr - buffer_end_;
      GOOGLE_DCHECK_LE(overrun, kSlopBytes);  // Guaranteed by parse loop.
      if (overrun ==
          limit_) {  //  No need to flip buffers if we ended on a limit.
--- 208,214 ----
    bool DoneWithCheck(const char** ptr, int d) {
      GOOGLE_DCHECK(*ptr);
      if (PROTOBUF_PREDICT_TRUE(*ptr < limit_end_)) return false;
!     size_t overrun = *ptr - buffer_end_;
      GOOGLE_DCHECK_LE(overrun, kSlopBytes);  // Guaranteed by parse loop.
      if (overrun ==
          limit_) {  //  No need to flip buffers if we ended on a limit.
***************
*** 217,223 ****
        if (overrun > 0 && next_chunk_ == nullptr) *ptr = nullptr;
        return true;
      }
!     auto res = DoneFallback(overrun, d);
      *ptr = res.first;
      return res.second;
    }
--- 217,223 ----
        if (overrun > 0 && next_chunk_ == nullptr) *ptr = nullptr;
        return true;
      }
!     auto res = DoneFallback(static_cast<int>(overrun), d);
      *ptr = res.first;
      return res.second;
    }
***************
*** 347,353 ****
    const char* AppendUntilEnd(const char* ptr, const A& append) {
      if (ptr - buffer_end_ > limit_) return nullptr;
      while (limit_ > kSlopBytes) {
!       int chunk_size = buffer_end_ + kSlopBytes - ptr;
        GOOGLE_DCHECK_GE(chunk_size, 0);
        append(ptr, chunk_size);
        ptr = Next();
--- 347,353 ----
    const char* AppendUntilEnd(const char* ptr, const A& append) {
      if (ptr - buffer_end_ > limit_) return nullptr;
      while (limit_ > kSlopBytes) {
!       size_t chunk_size = buffer_end_ + kSlopBytes - ptr;
        GOOGLE_DCHECK_GE(chunk_size, 0);
        append(ptr, chunk_size);
        ptr = Next();
***************
*** 428,434 ****
  template <uint32 tag>
  bool ExpectTag(const char* ptr) {
    if (tag < 128) {
!     return *ptr == tag;
    } else {
      static_assert(tag < 128 * 128, "We only expect tags for 1 or 2 bytes");
      char buf[2] = {static_cast<char>(tag | 0x80), static_cast<char>(tag >> 7)};
--- 428,434 ----
  template <uint32 tag>
  bool ExpectTag(const char* ptr) {
    if (tag < 128) {
!     return *ptr == static_cast<char>(tag);
    } else {
      static_assert(tag < 128 * 128, "We only expect tags for 1 or 2 bytes");
      char buf[2] = {static_cast<char>(tag | 0x80), static_cast<char>(tag >> 7)};

@gvollant
Copy link
Contributor Author

@gvollant gvollant commented Dec 29, 2020

@coryan @acozzette
hello, do you known if I need a specific procedure to get this pull request reviewed?

regards,gilles

@acozzette acozzette added release notes: yes c++ kokoro:run labels Dec 29, 2020
@@ -208,7 +208,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream {
bool DoneWithCheck(const char** ptr, int d) {
GOOGLE_DCHECK(*ptr);
if (PROTOBUF_PREDICT_TRUE(*ptr < limit_end_)) return false;
int overrun = *ptr - buffer_end_;
size_t overrun = *ptr - buffer_end_;
Copy link
Member

@acozzette acozzette Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this has to be a signed type because overrun can be negative. Maybe std::ptrdiff_t would be the right type.

Copy link
Contributor

@coryan coryan Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: auto overrun = *ptr - buffer_end_;

Copy link
Contributor Author

@gvollant gvollant Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally, I made a new commit with
int overrun = static_cast<int>(*ptr - buffer_end_);

overrun is used for compare with limit_ (which is int) and as first argument of DoneFallback (which expect int).

In visual studio 2019 x64 target, size_t are 64 bits and int are 32 bits
@acozzette acozzette merged commit 9647a7c into protocolbuffers:master Dec 30, 2020
54 checks passed
@acozzette
Copy link
Member

@acozzette acozzette commented Dec 30, 2020

Thanks, @gvollant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ cla: yes release notes: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants