Possible fix for #8734 #8735
Possible fix for #8734 #8735
Conversation
src/dsql/DsqlRequests.cpp
Outdated
| // If it succeed then the next record exists. | ||
|
|
||
| JRD_receive(tdbb, request, message->msg_number, outMsgLength, message_buffer.get()); | ||
| std::unique_ptr<UCHAR[]> message_buffer(new UCHAR[outMsgLength]); |
There was a problem hiding this comment.
Size of the message is known at this point, so usage of HalfStaticArray will just waste space on stack or cause stack overflow if this size is used as InlineCapacity.
There was a problem hiding this comment.
Do you think that outMsgLength could be used as InlineCapacity template argument ?
Looks like you not understand purpose and usage of HalfStaticArray.
There was a problem hiding this comment.
Do you think that
outMsgLengthcould be used asInlineCapacitytemplate argument ?
No.
Looks like you not understand purpose and usage of
HalfStaticArray.
Yes. Especially in this place which is executed exactly once per query.
+1 |
src/dsql/DsqlRequests.cpp
Outdated
| { | ||
| // Create a temp message buffer and try one more receive. | ||
| // If it succeed then the next record exists. | ||
| if (outMsgLength > 0) |
There was a problem hiding this comment.
I don't understand why there is check for outMsgLength > 0 - is it possible to be false when singleton == true ?
Even if out message with zero length is possible (???), what should happens for the singleton && outMsgLength == 0 case ?
This place is very unclear for me.
In v5 there was just check for singleton == true, btw
There was a problem hiding this comment.
Just in case. If the function was called with outMetadata == nullptr - just assume success. The difference from v5 is that buffers here are provided by application rather than of engine itself.
There was a problem hiding this comment.
Is it possible to have singleton == true && outMetadata == nullptr ?
In any case, why two nested if here ?
There was a problem hiding this comment.
Initially check for request's activity was out of inner if. Then I realized that some BLR code can intentionally run to the end without entering blr_send and an application aware of it can provide no output buffer.
If you are sure that I'm wrong feel free to change this piece later.
There was a problem hiding this comment.
I'm sure that this code is not clear. And you changed it this way.
Formally,
if (a) {
if (b) {
...
}
}
is the same as
if (a && b) {
...
}
thus I ask why you choose first one. If there is no real reason, change it.
If there is a reasons - provide it, and put in comments to avoid similar misunderstanding for the future readers.
No description provided.