[io] Additional checks to the properties of the TKey's buffer#21815
[io] Additional checks to the properties of the TKey's buffer#21815dpiparo wants to merge 1 commit intoroot-project:masterfrom
Conversation
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 966eafc. |
| return; | ||
| } | ||
|
|
||
| if (fNbytes < fKeylen) { |
There was a problem hiding this comment.
Those 2 numbers are unrelated and this condition is actually valid (fNbytes is the on-file size (compressed size) the data payload and those include the size of the key). If anything, we have an issue if:
if (fObjlen < fNbytes) {
There was a problem hiding this comment.
Isn't the TKey header always uncompressed? If so, this check looks correct to me.
There was a problem hiding this comment.
Isn't the TKey header always uncompressed?
yes.
If so, this check looks correct to me.
fKeyLen and fObjLen/fNbytes are still completely unrelated. The total size of the key on disk is fKeyLen + fNbytes. fNbytes can technically even be zero (that's silly but still legal/possible, I think).
There was a problem hiding this comment.
The documentation of fNbytes says: "number of bytes for the compressed object and key." fKeyLen is already included. It must be, because fNbytes is used to navigate along the linked list of keys.
There was a problem hiding this comment.
Annoyingly the documentations are inconsistent or unclear. TKey.h says:
Int_t fNbytes; ///< Number of bytes for the object on file
while your quote is from TKey.cxx.
And indeed in addition to the navigation you mention, the code substracts fKeyLen from fNbytes before comparing to fObjLen.
Thanks for clarifying this ! :)
No description provided.