Skip to content

Commit 5e0abd5

Browse files
committed
Merge TASK-063: drop accepted-but-ignored size_hint from http_response::pipe
2 parents d97a421 + 7b790e9 commit 5e0abd5

6 files changed

Lines changed: 29 additions & 25 deletions

File tree

RELEASE_NOTES.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,15 @@ and see the v2 replacement.
222222
verbose form for local development, call `.expose_credentials_in_logs(true)`
223223
on the `create_webserver` builder — this flag is intended for development
224224
only and must not be set in production deployments.
225+
- **`http_response::pipe` no longer accepts a `size_hint`.** Earlier v2
226+
work-in-progress builds exposed `pipe(int fd, std::size_t size_hint = 0)`
227+
with `size_hint` reserved for future use; the parameter was never
228+
consulted on the dispatch path. v2.0 ships the minimal `pipe(int fd)`
229+
signature (TASK-063, PRD §3.5 / `PRD-RSP-REQ-001`). Source callers
230+
passing a second argument must drop it; no behavioural change. There
231+
is no honoring path planned — `Content-Length` synthesis would lie
232+
when the pipe yields a different byte count, and libmicrohttpd's
233+
`MHD_create_response_from_pipe` takes no size.
225234

226235
## Threading
227236

specs/tasks/M7-v2-cleanup/TASK-063.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
`pipe(int fd, std::size_t size_hint = 0)` exposes `size_hint` in the public API but the dispatch path ignores it; a test (`test/unit/http_response_factories_test.cpp:253-266`) pins "accepted-but-ignored" as the contract. Decide whether to honor the hint (sizing the streaming buffer, `Content-Length` synthesis, or kernel hint via `posix_fadvise`) or remove it from the signature.
99

1010
**Action Items:**
11-
- [ ] Decide one of: (a) honor `size_hint` as the streaming chunk size and/or `Content-Length`, or (b) remove the parameter and the pinning test.
11+
- [x] Decide one of: (a) honor `size_hint` as the streaming chunk size and/or `Content-Length`, or (b) remove the parameter and the pinning test. — chose (b); rationale recorded in `.groundwork-plans/TASK-063-plan.md` (no `PRD-RSP-REQ` for streaming-size; MHD's `MHD_create_response_from_pipe` takes no size; `Content-Length` synthesis would lie).
1212
- [ ] If (a): wire `size_hint` into `detail::body_pipe` (or wherever the pipe body is materialised) and add a `Content-Length` header when the hint is nonzero and finite. Add a real integration test that pins the byte-count emitted on the wire matches the hint when the underlying fd produces exactly that many bytes.
13-
- [ ] If (b): drop `size_hint` from the public `pipe(int fd)` signature in `src/httpserver/http_response.hpp:159-161`, drop the corresponding default-arg in `src/http_response.cpp:409`, and convert the "accepted-but-ignored" test in `test/unit/http_response_factories_test.cpp:253-266` into a compile-fail or call-site update.
14-
- [ ] Update RELEASE_NOTES.md "Migration notes" if the signature changed (binary/source incompatibility).
13+
- [x] If (b): drop `size_hint` from the public `pipe(int fd)` signature in `src/httpserver/http_response.hpp:159-161`, drop the corresponding default-arg in `src/http_response.cpp:409`, and convert the "accepted-but-ignored" test in `test/unit/http_response_factories_test.cpp:253-266` into a compile-fail or call-site update.
14+
- [x] Update RELEASE_NOTES.md "Migration notes" if the signature changed (binary/source incompatibility).
1515

1616
**Dependencies:**
1717
- Blocked by: None
@@ -26,4 +26,4 @@
2626
**Related Requirements:** PRD-RSP-REQ-001 (factory by value), PRD §2 API minimalism
2727
**Related Decisions:** None new
2828

29-
**Status:** Backlog
29+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ TASK-093).
2727
| TASK-060 | Scope or remove file-scoped `-Warray-bounds` suppressions | HIGH | S | Done |
2828
| TASK-061 | Mechanical cleanup sweep — unfinished prose, orphan comments, stale doc refs | HIGH | S | Done |
2929
| TASK-062 | RFC-7616-compliant Digest auth response factory | HIGH | L | Done |
30-
| TASK-063 | Honor or remove `http_response::pipe` `size_hint` parameter | HIGH | S | Backlog |
30+
| TASK-063 | Honor or remove `http_response::pipe` `size_hint` parameter | HIGH | S | Done |
3131
| TASK-064 | Structured cookie type | HIGH | M | Backlog |
3232
| TASK-065 | RFC 5952 IPv6 zero-compression in `peer_address` | HIGH | S | Backlog |
3333
| TASK-066 | Runtime setter for hook alias slots | MED | M | Backlog |

src/http_response.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,7 @@ http_response http_response::iovec(std::span<const iovec_entry> entries) {
405405
return r;
406406
}
407407

408-
http_response http_response::pipe(int fd, std::size_t size_hint) {
409-
(void)size_hint; // reserved for future use
408+
http_response http_response::pipe(int fd) {
410409
http_response r;
411410
r.status_code_ = http::http_utils::http_ok;
412411
r.emplace_body<detail::pipe_body>(body_kind::pipe, fd);

src/httpserver/http_response.hpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,7 @@ class http_response final {
192192
// when the materialized MHD_Response is destroyed; if the response
193193
// is never materialized, the http_response's destructor closes
194194
// it. Callers MUST NOT close `fd` after handing it off.
195-
// `size_hint` is reserved for forward compatibility — currently
196-
// ignored, may be used to advise libmicrohttpd of payload size in
197-
// a future revision.
198-
[[nodiscard]] static http_response pipe(int fd,
199-
std::size_t size_hint = 0);
195+
[[nodiscard]] static http_response pipe(int fd);
200196

201197
// Construct an empty (no-payload) response. Defaults to 204
202198
// No Content, matching v1 empty_response. The optional `mhd_flags`

test/unit/http_response_factories_test.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -250,20 +250,20 @@ LT_BEGIN_AUTO_TEST(http_response_factories_suite, pipe_factory_kind)
250250
::close(fds[1]);
251251
LT_END_AUTO_TEST(pipe_factory_kind)
252252

253+
// Pin: pipe() must accept exactly one argument (the fd). Any future
254+
// reintroduction of a size_hint / chunk_size / Content-Length parameter
255+
// is a deliberate API change and must update this assertion. See
256+
// TASK-063 — rationale: an accepted-but-ignored parameter teaches
257+
// callers a lie; honoring it would require synthesising Content-Length
258+
// without ground truth from the pipe fd.
253259
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
254-
pipe_factory_size_hint_is_accepted_but_ignored)
255-
// size_hint is reserved for future use; callers may pass it without
256-
// observable effect today.
257-
int fds[2];
258-
int rc = ::pipe(fds);
259-
LT_ASSERT_EQ(rc, 0);
260-
{
261-
auto r = http_response::pipe(fds[0], /*size_hint=*/4096);
262-
LT_CHECK_EQ(static_cast<int>(r.kind()),
263-
static_cast<int>(body_kind::pipe));
264-
}
265-
::close(fds[1]);
266-
LT_END_AUTO_TEST(pipe_factory_size_hint_is_accepted_but_ignored)
260+
pipe_factory_signature_is_single_arg)
261+
using pipe_fn_t = http_response (*)(int);
262+
static_assert(std::is_same_v<decltype(&http_response::pipe), pipe_fn_t>,
263+
"http_response::pipe must take exactly one parameter "
264+
"(int fd); see TASK-063");
265+
LT_CHECK_EQ(true, true); // littletest needs at least one runtime check
266+
LT_END_AUTO_TEST(pipe_factory_signature_is_single_arg)
267267
#endif // !_WIN32
268268
269269
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)