test/bufferlist: Fixing corruption on ptr(ptr&& p) #7779
Conversation
|
👍 |
|
LGTM |
|
👍 |
ghost
self-assigned this
|
Should I do something to get it merged ? |
|
@ErwanAliasr1 no, we just need to run it with a rados qa test. |
|
@ErwanAliasr1 now that make check has been fixed, it would help if you could rebase and repush, just for the sake of having it clean. The "needs-qa" means that a run of teuthology tests need to happen to verify there is no regression. When a commit touches a central part of the Ceph code, this is a necessary precaution. It usually takes from one to four weeks for that verification to happen. So you can add a reminder to ping in four weeks from now if nothing else happens. Other than this, you're good :-) |
The bufferlist is trying to compare two strings by doing :
str.compare(0, str.size(), ptr.c_str())
This have been introduced recently in commit 41c3dc2.
The issue in that test is not providing the amount of char to compare but only
the size of 'str'.
As a result, it does compare a null-terminated string (str) with a
non-terminated buffer (ptr).
The issue was discoverd by having on a test where
- str="XXXXXXXXXXXXXXXXX"
but
- ptr="XXXXXXXXXXXXXXXXXriptor".
The associated assertion failed and broke the test.
This patch is fixing the issue by using the memcmp() call as per 'ptr(const
ptr& p, unsigned o, unsigned l)' test.
The simple solution is to test the buffer at the byte level and only for the
n-bytes of the initial string.
That change makes the test passing.
Regarding the initial code, that's pretty a big chance that no-one hit that
before.
Signed-off-by: Erwan Velu <erwan@redhat.com>
ErwanAliasr1
force-pushed
the
evelu-check-bufferlist
branch
from
6e02ad3
to
419cbab
Compare
|
@dachary rebased & repushed. |
|
already fixed by 7723d29 |
|
@liewegas : In this commit,if ptr.length() is shorter than str.size() then we do pass while it's incorrect :( |
|
This std::string compare variant we switched to (int compare (size_t pos, size_t len, const char* s, size_t n) const) in commit 7723d29 does this, right? |
|
I pushed #7955 to explain & solve the issue I see with this way to fix it. |
The bufferlist is trying to compare two strings by doing :
str.compare(0, str.size(), ptr.c_str())
This have been introduced recently in commit 41c3dc2.
The issue in that test is not providing the amount of char to compare but only
the size of 'str'.
As a result, it does compare a null-terminated string (str) with a
non-terminated buffer (ptr).
The issue was discoverd by having on a test where
but
The associated assertion failed and broke the test.
This patch is fixing the issue by using the memcmp() call as per 'ptr(const
ptr& p, unsigned o, unsigned l)' test.
The simple solution is to test the buffer at the byte level and only for the
n-bytes of the initial string.
That change makes the test passing.
Regarding the initial code, that's pretty a big chance that no-one hit that
before.
Signed-off-by: Erwan Velu erwan@redhat.com