Re: [PATCH 02/11] Support GIT_TEST_GETTEXT_POISON=rot13

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 12, 2021, 19:37 UTC
Message-ID: <xmqqmtxe3q0u.fsf@gitster.c.googlers.com>
In-Reply-To: <ab7832fc7ac783bc8f8c10014fc9747128fc37bc.1610441263.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> +const char *gettext_maybe_rot13(const char *msgid)
> +{
> +...
> +	while (*msgid) {
> +		const char *p = strchrnul(msgid, '%'), *spec;
> +
> +		while (*p && p[1] == '%')
> +			p = strchrnul(p + 2, '%');

We are at '%', and while the next is '%' (i.e. a literal '%' is asked), we skip these two bytes and then we go back to the equivalent of the initialization of 'p'. Effects? We leave the loop when we are at the end of the string, or at '%' that is not followed by a '%'.

It would have been easier to understand what is going on in the loop if there weren't duplicated strchrnul() call, perhaps

	const char *p = msgid;
	while (*(p = strchrnul(p, '%')) == '%' && p[1] == '%')
        	p += 2;

That reads "find '%' and if the next char is also '%', skip these two and redo the loop", which would be equivalent. And it would be much easier to follow the logic.

> +		if (p != msgid) {

And if 'p' is different (actually we know p is the same as, or always ahead of, msgid, so "if (msgid < p)" would be easier to follow for readers) ...

> +			strbuf_addstr(buf, "<rot13>");
> +			while (p != msgid)

Ditto.

> +				strbuf_addch(buf, do_rot13(*(msgid++)));
> +			strbuf_addstr(buf, "</rot13>");

... we add a section of obfuscated output enclosed with an xml looking marker.

> +		}
> +
> +		if (!*p)
> +			break;

And if we are at the end, we are done.

> +		spec = strpbrk(p + 1, "diouxXeEfFgGaAcsCSpnm%");
> +		if (!spec)
> +			BUG("Unrecognized format string: %s", p);

It is a bit surprising that things like "%.*d" and "%.7f" do not appear in our translatable strings (or perhaps this is one of the places why the series is marked RFH and is not complete?).

Show 6 quoted lines

> +		strbuf_add(buf, p, spec + 1 - p);
> +		msgid = spec + 1;
> +	}
> +
> +	return buf->buf;
> +}

Anyway, thanks for a fun reading.