Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/733#discussion_r68171726
--- Diff: proxy/http2/Http2ClientSession.h ---
@@ -94,63 +89,60 @@ class Http2Frame
{
this->ioblock = new_IOBufferBlock();
this->ioblock->alloc(index);
- memcpy(this->ioblock->start(), this->hdr.raw, sizeof(this->hdr.raw));
- this->ioblock->fill(sizeof(this->hdr.raw));
-
- http2_parse_frame_header(make_iovec(this->ioblock->start(),
HTTP2_FRAME_HEADER_LEN), this->hdr.cooked);
}
- // Return the writeable buffer space.
+ // Return the writeable buffer space for frame payload
IOVec
write()
{
return make_iovec(this->ioblock->end(), this->ioblock->write_avail());
}
- // Once the frame has been serialized, update the length.
+ // Once the frame has been serialized, update the payload length of
frame header.
void
finalize(size_t nbytes)
{
if (this->ioblock) {
ink_assert((int64_t)nbytes <= this->ioblock->write_avail());
this->ioblock->fill(nbytes);
- this->hdr.cooked.length = this->ioblock->size() -
HTTP2_FRAME_HEADER_LEN;
- http2_write_frame_header(this->hdr.cooked,
make_iovec(this->ioblock->start(), HTTP2_FRAME_HEADER_LEN));
+ this->hdr.length = this->ioblock->size();
}
}
void
xmit(MIOBuffer *iobuffer)
{
- if (ioblock) {
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(hdr, make_iovec(buf));
+ iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ // It could be empty (e.g. SETTINGS frame with ACK flag)
+ if (ioblock && ioblock->read_avail() > 0) {
iobuffer->append_block(this->ioblock.get());
- } else {
- iobuffer->write(this->hdr.raw, sizeof(this->hdr.raw));
}
}
int64_t
size()
{
if (ioblock) {
- return ioblock->size();
+ return HTTP2_FRAME_HEADER_LEN + ioblock->size();
} else {
- return sizeof(this->hdr.raw);
+ return HTTP2_FRAME_HEADER_LEN;
}
}
private:
Http2Frame(Http2Frame &); // noncopyable
Http2Frame &operator=(const Http2Frame &); // noncopyable
- Ptr<IOBufferBlock> ioblock;
IOBufferReader *ioreader;
- union {
- Http2FrameHeader cooked;
- uint8_t raw[HTTP2_FRAME_HEADER_LEN];
- } hdr;
+ Http2FrameHeader hdr; // frame header
+ Ptr<IOBufferBlock> ioblock; // frame payload
--- End diff --
Bikeshedding, but it would be nicer to keep ``ioreader`` down here near
``ioblock`` :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
