On Fri, Mar 06, 2015 at 09:57:22AM +0100, René Scharfe wrote:
> Convert hostname, canon_hostname, ip_address and tcp_port to strbuf.
> This allows to get rid of the helpers strbuf_addstr_or_null() and STRARG
> because a strbuf always represents a valid (initially empty) string.
> sanitize_client() becomes unused and is removed as well.
Makes sense. I had a feeling that we might have cared about NULL versus
the empty string somewhere, but I did not see it in the patch below, so
I think it is fine.
> -static char *sanitize_client(const char *in)
> -{
> - struct strbuf out = STRBUF_INIT;
> - sanitize_client_strbuf(&out, in);
> - return strbuf_detach(&out, NULL);
> -}
Not a big deal, but do we want to rename sanitize_client_strbuf to
sanitize_client? It only had the unwieldy name to distinguish it from
this one.
> if (port) {
> - free(tcp_port);
> - tcp_port = sanitize_client(port);
> + strbuf_reset(&tcp_port);
> + sanitize_client_strbuf(&tcp_port, port);
The equivalent of free() is strbuf_release(). I think it is reasonable
to strbuf_reset here, since we are about to write into it again anyway
(though I doubt it happens much in practice, since that would imply
multiple `host=` segments sent by the client). But later...
> - free(hostname);
> - free(canon_hostname);
> - free(ip_address);
> - free(tcp_port);
> - hostname = canon_hostname = ip_address = tcp_port = NULL;
> + strbuf_reset(&hostname);
> + strbuf_reset(&canon_hostname);
> + strbuf_reset(&ip_address);
> + strbuf_reset(&tcp_port);
These probably want to all be strbuf_release(). Again, I doubt it
matters much because this is a forked daemon serving only a single
request (so they'll get freed by the OS soon anyway), but I think
freeing the memory here follows the original intent.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html