io: ReadVersion should always take both position and byte count

Author: pcanal

io/io/src/TBufferFile.cxx

@@ -3007,14 +3007,15 @@ void TBufferFile::SkipVersion(const TClass *cl)
 
 Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass *cl)
 {
+   assert((!startpos && !bcnt) || (startpos && bcnt)); // both or none should be set
+
    Version_t version;
 
    if (startpos) {
       // before reading object save start position
       auto full_startpos = fBufCur - fBuffer;
       *startpos = full_startpos <= kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition;
-      if (bcnt)
-         fByteCountStack.push_back(full_startpos);
+      fByteCountStack.push_back(full_startpos);
    }
 
    // read byte count (older files don't have byte count)
@@ -3039,20 +3040,19 @@ Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass
       v.cnt = 0;
    }
    if (bcnt) {
+      // We also have (asserted) that (startpos != nullptr)
       if (!v.cnt) {
          // no byte count stored
          *bcnt = 0;
-         if (startpos) // Undo the push_back only if it happened.
-            fByteCountStack.pop_back();
+         fByteCountStack.pop_back();
       } else {
          *bcnt = (v.cnt & ~kByteCountMask);
          if (*bcnt == 0) {
             // The byte count was stored but is zero, this means the data
             // did not fit and thus we stored it in 'fByteCounts' instead.
             // Mark this case by setting startpos to kOverflowCount.
             *bcnt = kOverflowCount;
-            if (startpos)
-               *startpos = kOverflowPosition;
+            *startpos = kOverflowPosition;
          }
       }
    }
@@ -3146,14 +3146,15 @@ Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass
 
 Version_t TBufferFile::ReadVersionNoCheckSum(UInt_t *startpos, UInt_t *bcnt)
 {
+   assert((!startpos && !bcnt) || (startpos && bcnt)); // both or none should be set
+
    Version_t version;
 
    if (startpos) {
       // before reading object save start position
       auto full_startpos = fBufCur - fBuffer;
       *startpos = full_startpos < kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition;
-      if (bcnt)
-         fByteCountStack.push_back(full_startpos);
+      fByteCountStack.push_back(full_startpos);
    }
 
    // read byte count (older files don't have byte count)
@@ -3181,17 +3182,15 @@ Version_t TBufferFile::ReadVersionNoCheckSum(UInt_t *startpos, UInt_t *bcnt)
       if (!v.cnt) {
          // no byte count stored
          *bcnt = 0;
-         if (startpos) // Undo the push_back only if it happened.
-            fByteCountStack.pop_back();
+         fByteCountStack.pop_back();
       } else {
          *bcnt = (v.cnt & ~kByteCountMask);
          if (*bcnt == 0) {
             // The byte count was stored but is zero, this means the data
             // did not fit and thus we stored it in 'fByteCounts' instead.
             // Mark this case by setting startpos to kOverflowCount.
             *bcnt = kOverflowCount;
-            if (startpos)
-               *startpos = kOverflowPosition;
+            *startpos = kOverflowPosition;
          }
       }
    }