From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 22 Aug 2012 16:43:38 GMT
Subject: Re: [PATCH] Support generate poison .mo files for testing
Message-ID: <7vtxvuand1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1345613246-4053-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> 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?