Early return when size == 0 #2009
Conversation
| @@ -1211,6 +1211,9 @@ int IOBuf::append_user_data(void* data, size_t size, void (*deleter)(void*)) { | |||
| LOG(FATAL) << "data_size=" << size << " is too large"; | |||
| return -1; | |||
| } | |||
| if (!size) { | |||
| return 0; | |||
There was a problem hiding this comment.
Maybe we need to call deleter here?
There was a problem hiding this comment.
Was deleter called in the original implementation when size is 0? It's better to keep the semantics as before.
There was a problem hiding this comment.
I still think size==0 should return -1 rather than 0. If it is failed, it means IOBuf does not take this Block at all. It is the caller's responsibility to delete the buffer.
There was a problem hiding this comment.
@Tuvie Makes sense. Does return -1 break backward compatibility?
Also return 0 && call deleter means that IOBuf takes the responsibility to free the buff. This is consistent with the behavior before this PR.
There was a problem hiding this comment.
@Tuvie Both free and delete can operate on nullptr. Do we have any other pitfalls calling deleter here?
There was a problem hiding this comment.
Shouldn't we move the deleter null check and assignment below before this change? Otherwise the data may be leaked if user doesn't pass deleter.
There was a problem hiding this comment.
Yes, we need to keep the semantics.
|
LGTM |
What problem does this PR solve?
Issue Number:
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects(性能影响):
Breaking backward compatibility(向后兼容性):
Check List:
As pointed out by @Tuvie #1995 (comment), when
IOBuf::append_user_datais called with size 0, we can do early return. This avoids unnecessary block memory allocation in the later.