public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Elia Pinto <gitter.spiros@gmail•com>
Cc: git@vger•kernel.org, tboegi@web•de, ramsay@ramsayjones•plus.com,
	sunshine@sunshineco•com, peff@peff•net
Subject: Re: [PATCHv5 1/2] http.c: implement the GIT_TRACE_CURL environment variable
Date: Mon, 02 May 2016 11:59:14 -0700	[thread overview]
Message-ID: <xmqq60uwthfh.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160502142813.50868-2-gitter.spiros@gmail.com> (Elia Pinto's message of "Mon, 2 May 2016 14:28:12 +0000")

Elia Pinto <gitter.spiros@gmail•com> writes:

> +static void curl_dump(const char *text, unsigned char *ptr, size_t size, int nopriv)
> +{
> +	size_t i;
> +	struct strbuf out = STRBUF_INIT;
> +	unsigned int width = 80;
> +
> +	strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
> +		text, (long)size, (long)size);
> +	trace_strbuf(&trace_curl, &out);
> +
> +	for (i = 0; i < size; i += width) {
> +		...
> +		for (w = 0; (w < width) && (i + w < size); w++) {
> +			if ((i + w + 1 < size) && ptr[i + w] == '\r'
> +			    && ptr[i + w + 1] == '\n') {
> +				i += (w + 2 - width);
> +				break;
> +			}
> +			strbuf_addch(&out, (ptr[i + w] >= 0x20)
> +				&& (ptr[i + w] < 0x80) ? ptr[i + w] : '.');
> +			if ((i + w + 2 < size)
> +			    && ptr[i + w + 1] == '\r'
> +			    && ptr[i + w + 2] == '\n') {
> +				i += (w + 3 - width);
> +				break;
> +			}
> +		}
> +		...
> +	}
> +	strbuf_release(&out);
> +}

There is no change in this hard-to-read double-loop since the
previous round?

> +int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp)
> +{
> +	const char *text;
> +	(void)handle;		/* prevent compiler unused parameter warning if checked */
> +	(void)userp;		/* prevent compiler unused parameter warning if checked */

I really do not want to see these casts.  Unused parameters are
perfectly normal in a codebase with callback functions, no?  I do
not think these are the first occurrences of unused parameters in
our codebase, and I do not think we have such cast to void to them.
Why add this ugliness only to here?

next prev parent reply	other threads:[~2016-05-02 18:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-02 14:28 [PATCHv5 0/2] Implement the GIT_TRACE_CURL environment variable Elia Pinto
2016-05-02 14:28 ` [PATCHv5 1/2] http.c: implement " Elia Pinto
2016-05-02 17:03   ` Ramsay Jones
2016-05-02 18:15   ` Jeff King
2016-05-02 18:59   ` Junio C Hamano [this message]
2016-05-02 19:25     ` Jeff King
2016-05-02 19:15   ` Junio C Hamano
2016-05-02 14:28 ` [PATCHv5 2/2] imap-send.c: introduce " Elia Pinto
2016-05-02 18:13 ` [PATCHv5 0/2] Implement " Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq60uwthfh.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitter.spiros@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=ramsay@ramsayjones$(echo .)plus.com \
    --cc=sunshine@sunshineco$(echo .)com \
    --cc=tboegi@web$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox