Skip to content

Commit 4277635

Browse files
jasnellMylesBorins
authored andcommitted
http2: clean up Http2Settings
Use of a MaybeStackBuffer was just silly. Fix a long standing todo Reduce code duplication a bit. PR-URL: #19400 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 42b6d80 commit 4277635

File tree

2 files changed

+16
-54
lines changed

2 files changed

+16
-54
lines changed

src/node_http2.cc

+15-47
Original file line numberDiff line numberDiff line change
@@ -190,60 +190,28 @@ Http2Options::Http2Options(Environment* env) {
190190
}
191191

192192
void Http2Session::Http2Settings::Init() {
193-
entries_.AllocateSufficientStorage(IDX_SETTINGS_COUNT);
194193
AliasedBuffer<uint32_t, v8::Uint32Array>& buffer =
195194
env()->http2_state()->settings_buffer;
196195
uint32_t flags = buffer[IDX_SETTINGS_COUNT];
197196

198197
size_t n = 0;
199198

200-
if (flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) {
201-
uint32_t val = buffer[IDX_SETTINGS_HEADER_TABLE_SIZE];
202-
DEBUG_HTTP2SESSION2(session_, "setting header table size: %d\n", val);
203-
entries_[n].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
204-
entries_[n].value = val;
205-
n++;
199+
#define GRABSETTING(N, trace) \
200+
if (flags & (1 << IDX_SETTINGS_##N)) { \
201+
uint32_t val = buffer[IDX_SETTINGS_##N]; \
202+
DEBUG_HTTP2SESSION2(session_, "setting " trace ": %d\n", val); \
203+
entries_[n++] = \
204+
nghttp2_settings_entry {NGHTTP2_SETTINGS_##N, val}; \
206205
}
207206

208-
if (flags & (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS)) {
209-
uint32_t val = buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS];
210-
DEBUG_HTTP2SESSION2(session_, "setting max concurrent streams: %d\n", val);
211-
entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS;
212-
entries_[n].value = val;
213-
n++;
214-
}
215-
216-
if (flags & (1 << IDX_SETTINGS_MAX_FRAME_SIZE)) {
217-
uint32_t val = buffer[IDX_SETTINGS_MAX_FRAME_SIZE];
218-
DEBUG_HTTP2SESSION2(session_, "setting max frame size: %d\n", val);
219-
entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_FRAME_SIZE;
220-
entries_[n].value = val;
221-
n++;
222-
}
223-
224-
if (flags & (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE)) {
225-
uint32_t val = buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE];
226-
DEBUG_HTTP2SESSION2(session_, "setting initial window size: %d\n", val);
227-
entries_[n].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
228-
entries_[n].value = val;
229-
n++;
230-
}
231-
232-
if (flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) {
233-
uint32_t val = buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE];
234-
DEBUG_HTTP2SESSION2(session_, "setting max header list size: %d\n", val);
235-
entries_[n].settings_id = NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE;
236-
entries_[n].value = val;
237-
n++;
238-
}
207+
GRABSETTING(HEADER_TABLE_SIZE, "header table size");
208+
GRABSETTING(MAX_CONCURRENT_STREAMS, "max concurrent streams");
209+
GRABSETTING(MAX_FRAME_SIZE, "max frame size");
210+
GRABSETTING(INITIAL_WINDOW_SIZE, "initial window size");
211+
GRABSETTING(MAX_HEADER_LIST_SIZE, "max header list size");
212+
GRABSETTING(ENABLE_PUSH, "enable push");
239213

240-
if (flags & (1 << IDX_SETTINGS_ENABLE_PUSH)) {
241-
uint32_t val = buffer[IDX_SETTINGS_ENABLE_PUSH];
242-
DEBUG_HTTP2SESSION2(session_, "setting enable push: %d\n", val);
243-
entries_[n].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH;
244-
entries_[n].value = val;
245-
n++;
246-
}
214+
#undef GRABSETTING
247215

248216
count_ = n;
249217
}
@@ -289,7 +257,7 @@ Local<Value> Http2Session::Http2Settings::Pack() {
289257
ssize_t ret =
290258
nghttp2_pack_settings_payload(
291259
reinterpret_cast<uint8_t*>(Buffer::Data(buf)), len,
292-
*entries_, count_);
260+
&entries_[0], count_);
293261
if (ret >= 0)
294262
return buf;
295263
else
@@ -344,7 +312,7 @@ void Http2Session::Http2Settings::RefreshDefaults(Environment* env) {
344312
void Http2Session::Http2Settings::Send() {
345313
Http2Scope h2scope(session_);
346314
CHECK_EQ(nghttp2_submit_settings(**session_, NGHTTP2_FLAG_NONE,
347-
*entries_, length()), 0);
315+
&entries_[0], count_), 0);
348316
}
349317

350318
void Http2Session::Http2Settings::Done(bool ack) {

src/node_http2.h

+1-7
Original file line numberDiff line numberDiff line change
@@ -1207,12 +1207,6 @@ class Http2Session::Http2Settings : public AsyncWrap {
12071207
void Send();
12081208
void Done(bool ack);
12091209

1210-
size_t length() const { return count_; }
1211-
1212-
nghttp2_settings_entry* operator*() {
1213-
return *entries_;
1214-
}
1215-
12161210
// Returns a Buffer instance with the serialized SETTINGS payload
12171211
Local<Value> Pack();
12181212

@@ -1229,7 +1223,7 @@ class Http2Session::Http2Settings : public AsyncWrap {
12291223
Http2Session* session_;
12301224
uint64_t startTime_;
12311225
size_t count_ = 0;
1232-
MaybeStackBuffer<nghttp2_settings_entry, IDX_SETTINGS_COUNT> entries_;
1226+
nghttp2_settings_entry entries_[IDX_SETTINGS_COUNT];
12331227
};
12341228

12351229
class ExternalHeader :

0 commit comments

Comments
 (0)