Skip to content

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

Merged
acozzette merged 1 commit intoprotocolbuffers:masterfrom
gvollant:bugfix/gvollant_warning_vs2019x64
Dec 30, 2020
Merged

Fix somes warning when compiling with Visual Studio 2019 on x64 target#8125
acozzette merged 1 commit intoprotocolbuffers:masterfrom
gvollant:bugfix/gvollant_warning_vs2019x64

Conversation

@gvollant
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor Author

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

regards,gilles

Comment thread src/google/protobuf/parse_context.h Outdated
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
Copy Markdown

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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
@acozzette
Copy link
Copy Markdown

Thanks, @gvollant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants