Re: [Nbd] [PATCHv2 5/6] Add TLS support to server
- To: Alex Bligh <alex@...872...>
- Cc: "nbd-general@lists.sourceforge.net" <nbd-general@lists.sourceforge.net>
- Subject: Re: [Nbd] [PATCHv2 5/6] Add TLS support to server
- From: Wouter Verhelst <w@...112...>
- Date: Tue, 12 Apr 2016 16:04:54 +0200
- Message-id: <20160412140454.GD15484@...3...>
- In-reply-to: <1460394939-24868-6-git-send-email-alex@...872...>
- References: <1460394939-24868-1-git-send-email-alex@...872...> <1460394939-24868-6-git-send-email-alex@...872...>
On Mon, Apr 11, 2016 at 06:15:38PM +0100, Alex Bligh wrote:
[...]
> +#ifdef WITH_GNUTLS
[...]
> +#else
> +
> + send_reply(opt, *net, NBD_REP_ERR_UNSUP, 0, NULL);
NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM, perhaps).
You should think of NBD_REP_ERR_INVALID as 4xx errors (i.e., "you're
doing it wrong"), and of NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM) as
5xx errors ("something went wrong on my end").
NBD_REP_ERR_UNSUP really is only meant for "I don't know what the ****
you're talking about". It should only be referenced just once in a
server (in a "default:" case of a switch statement)
[...]
> - sock_flags_old = fcntl(net, F_GETFL, 0);
> - if (sock_flags_old == -1) {
> - msg(LOG_ERR, "Failed to get socket flags");
> - goto handler_err;
> - }
> -
> - sock_flags_new = sock_flags_old & ~O_NONBLOCK;
> - if (sock_flags_new != sock_flags_old &&
> - fcntl(net, F_SETFL, sock_flags_new) == -1) {
> - msg(LOG_ERR, "Failed to set socket to blocking mode");
> - goto handler_err;
> - }
> + if (set_nonblocking(client->net, 0) < 0) {
> + msg(LOG_ERR, "Failed to set socket to blocking mode");
> + goto handler_err;
> + }
Some whitespace errors there.
> if (set_peername(net, client)) {
> msg(LOG_ERR, "Failed to set peername");
> @@ -2241,11 +2375,15 @@ handle_modern_connection(GArray *const servers, const int sock)
>
> msg(LOG_INFO, "Starting to serve");
> serveconnection(client);
> + close (net);
> + if (client->net != net)
> + close (client->net);
Probably safer to have a block here, rather than a single line?
[...]
--
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12
Reply to: