Skip to content

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

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

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

regards,gilles

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
Contributor

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

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

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
Contributor

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