From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:49355)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1aRgXN-0007J9-1C
	for qemu-devel@nongnu.org; Fri, 05 Feb 2016 08:35:01 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1aRgXJ-00065m-1D
	for qemu-devel@nongnu.org; Fri, 05 Feb 2016 08:35:00 -0500
Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:35402)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1aRgXI-00065d-Dw
	for qemu-devel@nongnu.org; Fri, 05 Feb 2016 08:34:56 -0500
Received: by mail-wm0-x22d.google.com with SMTP id r129so27000270wmr.0
	for <qemu-devel@nongnu.org>; Fri, 05 Feb 2016 05:34:56 -0800 (PST)
References: <1454597781-18115-1-git-send-email-alex.bennee@linaro.org>
	<1454597781-18115-6-git-send-email-alex.bennee@linaro.org>
	<56B3D01C.9080405@twiddle.net> <56B3DF84.1010109@redhat.com>
From: Alex =?utf-8?Q?Benn=C3=A9e?= <alex.bennee@linaro.org>
In-reply-to: <56B3DF84.1010109@redhat.com>
Date: Fri, 05 Feb 2016 13:34:53 +0000
Message-ID: <87mvrfnuv6.fsf@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH v5 5/9] qemu-log: support simple pid
	substitution in logfile
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, dgilbert@redhat.com, crosthwaitepeter@gmail.com, pbonzini@redhat.com, aurelien@aurel32.net, Richard Henderson <rth@twiddle.net>


Eric Blake <eblake@redhat.com> writes:

> On 02/04/2016 03:26 PM, Richard Henderson wrote:
>> On 02/05/2016 01:56 AM, Alex Bennée wrote:
>>> +    if (g_strrstr(filename, "%d")) {
>>> +        /* if we are going to format this we'd better validate first */
>>> +        if (g_regex_match_simple("^[^%]+%d[^%]+$", filename, 0, 0)) {
>>
>> Why g_strrstr instead of strstr?  There should be only one, so why look
>> for the last?

Yeah, my fault for using the glib functions, I guess
g_strstr_len(filename, -1, "%d") would be the glib equivalent for
strstr.

> For that matter, why use a heavyweight regex, when you can achieve the
> same validation with the faster:
>
> char *tmp = strchr(filename, '%');
> if (tmp) {
>     if (tmp[1] != 'd' || strchr(tmp + 2, '%')) {
>         ...report invalid string
>     }

For option parsing I'm not too worried about speed. At least a regex
gives the explicit format we expect (for those that read regex). I guess
I can do it manually if preferred.

--
Alex Bennée