Re: [PATCH v6 04/12] test-http-server: add stub HTTP server test helper
- From
Matthew John Cheetham <mjcheetham@outlook.com>
- Date
- Jan 20, 2023, 22:05 UTC
- Message-ID
- <AS2PR03MB9815437B9BE892A9F0AC564BC0C59@AS2PR03MB9815.eurprd03.prod.outlook.com>
- In-Reply-To
- <230118.86k01kxfr7.gmgdl@evledraar.gmail.com>
On 2023-01-18 03:04, Ævar Arnfjörð Bjarmason wrote:
Show 42 quoted lines
>
> On Wed, Jan 18 2023, Matthew John Cheetham via GitGitGadget wrote:
>
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
>> [...]
>> +enum worker_result {
>> + /*
>> + * Operation successful.
>> + * Caller *might* keep the socket open and allow keep-alive.
>> + */
>> + WR_OK = 0,
>> [...]
>> + enum worker_result wr = WR_OK;
>> +
>> + if (client_addr)
>> + loginfo("Connection from %s:%s", client_addr, client_port);
>> +
>> + set_keep_alive(0, logerror);
>> +
>> + while (1) {
>> + if (write_in_full(STDOUT_FILENO, response, strlen(response)) < 0) {
>> + logerror("unable to write response");
>> + wr = WR_IO_ERROR;
>> + }
>> +
>> + if (wr != WR_OK)
>> + break;
>> + }
>> +
>> + close(STDIN_FILENO);
>> + close(STDOUT_FILENO);
>> +
>> + return !!(wr & WR_IO_ERROR);
>> +}
>
> We have cases where we assign "0" to a bitfield-looking structure like
> this, but only in cases where we're planning to use it as a boolean too.
>
> Or, in other cases where we want some to be explicitly <-1.
>
> Here though we're adding a mixed "OK" and error use, which seems a bit
> odd. Shouldn't we pick one or the other?You make a fair point about bitfields vs simple integer values. This was a holdover from previous early hacking on this work where I had the bitfield serve as a way to communicate the aspects of "does this count as an error?" and "should we close the connection?".
Upon second thought, I think just simple integer values would be fine as really only an "OK" and "HANGUP" are non-errors (the latter being the case that the client gracefully ended the connection without an error and we should exit).
Check for my next iteration for a rework on these `worker_result` values.
> So far (maybe in later commits?) nothing uses WR_HANGUP, and oddly we > also use the bitfield-looking thing as a return value from main()....
We don't use the `enum worker_result` values as a return from `main`. We only ever return 0 or 1 as we `return worker()` from `main`, and the only `return` from `worker()` is `!!(wr & WR_IO_ERROR)` - 1 if we have `WR_IO_ERROR` set, otherwise 0.
Thanks, Matthew