Subject: Re: [PATCH] Support generate poison .mo files for testing

From: Junio C Hamano <[email-masked]>
Date: Wed, 22 Aug 2012 09:43:38 -0700

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> +static void translate(const char *msg, struct strbuf *buf)
> +{
> +	const char *end = msg + strlen(msg);
> +	const char *text = "* GETTEXT POISON *";
> +	int text_len = strlen(text);
> +	int t = 0;
> +
> +	strbuf_reset(buf);
> +	/* preserve \n and printf format specifiers because msgfmt
> +	   barfs otherwise. */
> +	while (msg < end) {
> +		/* printf specifiers and shell variables, it's a quite
> +		   relax check */
> +		if ((*msg == '%' || *msg == '$') && msg+1 < end) {
> +			strbuf_addch(buf, *msg++);
> +			do
> +			       strbuf_addch(buf, *msg);
> +			while (msg < end && !isspace(*msg++));

Aside from the Style:

        do {
                ...
        } while ();

why are you special casing a run of non-blank letters that begin
with a dollar sign (swapping two ints is done with "%2$d %1$d", a
percent still at the beginning, so there must be something else I am
missing)?

Also why do you stop at isspace()?  Isn't a " " (space) a flag that
means "If the first character of a signed conversion is not a sign
or if a signed conversion results in no characters, a <space> shall
be prefixed to the result."

As the flags, min-width, precision, and length do not share the same
character as the conversion that has to come at the end, I think you
only want to do something like

	/*
         * conversion specifier characters, taken from:
         * http://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html
         */
        static const char printf_conversion[] = "diouxXfFeEgGaAcspnCS%";

        ...

        while (msg < end) {
                if (*msg == '%') {
                        strbuf_addch(buf, *msg++);
                        while (msg < end) {
                                int ch = *msg++;
                                strbuf_addch(buf, ch);
                                if (strchr(printf_conversion, ch))
                                        break;
                        }
                        /* copied the printf part literally */
                        continue;
		}
		... keep \n ...
                ... muck with string ...
	}

perhaps?