New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bufferlist: cleanup semantical wrong for bufferlist::append #12247

Merged
merged 1 commit into from Jan 16, 2017

Conversation

Projects
None yet
2 participants
@YankunLi
Contributor

YankunLi commented Dec 1, 2016

Signed-off-by: Yankun Li lioveni99@gmail.com

@@ -1791,7 +1791,7 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
if (gap > len) gap = len;
//cout << "append first char is " << data[0] << ", last char is " << data[len-1] << std::endl;
append_buffer.append(data, gap);
append(append_buffer, append_buffer.end() - gap, gap); // add segment to the list
append(append_buffer, append_buffer.length() - gap, gap); // add segment to the list

This comment has been minimized.

@tchaikov

tchaikov Dec 1, 2016

Contributor

@YankunLi this issue impacts both hammer and jewel, could you file a ticket on tracker? and put more details in the commit message? then add a test in src/test/bufferlist.cc. also, please add a "Fixes:" line at end of your commit message to connect this commit to the tracker ticket.

this is just semantically wrong. as we never call append_buffer.set_offset(), so the append_buffer._offset is always 0. in other words, append_buffer.length() == append_buffer.end().

@tchaikov tchaikov modified the milestone: kraken Dec 1, 2016

@tchaikov tchaikov added cleanup and removed bug fix labels Dec 1, 2016

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Dec 1, 2016

@YankunLi could you update the commit message, to note that this is more a cleanup than a fix?

other than the commit message, lgtm.

@tchaikov tchaikov self-assigned this Dec 1, 2016

@YankunLi

This comment has been minimized.

Contributor

YankunLi commented Dec 1, 2016

@tchaikov OK, thinks,

bufferlist: cleanup semantical wrong for bufferlist::append
this is just semantically wrong. as append_buffer.set_offset() was
called, so append_buffer._off is always 0. in other words,
append_buffer.length() == append_buffer.end(). using append_buffer.
length() is better in here.

Signed-off-by: Yankun Li <lioveni99@gmail.com>

@YankunLi YankunLi changed the title from bufferlist: repair the error of parameter for bufferlist::append to bufferlist: cleanup semantical wrong for bufferlist::append Dec 1, 2016

@tchaikov tchaikov merged commit d644f91 into ceph:master Jan 16, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment