From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTZ2d-0001s7-KT for qemu-devel@nongnu.org; Wed, 10 Feb 2016 12:59:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTZ2a-0001e9-Ej for qemu-devel@nongnu.org; Wed, 10 Feb 2016 12:59:03 -0500 Received: from mail-qk0-x242.google.com ([2607:f8b0:400d:c09::242]:36481) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTZ2a-0001e5-5i for qemu-devel@nongnu.org; Wed, 10 Feb 2016 12:59:00 -0500 Received: by mail-qk0-x242.google.com with SMTP id e124so902521qkc.3 for ; Wed, 10 Feb 2016 09:58:59 -0800 (PST) Sender: Richard Henderson References: <1454597781-18115-1-git-send-email-alex.bennee@linaro.org> <1454597781-18115-7-git-send-email-alex.bennee@linaro.org> <56B3D9D6.2040600@twiddle.net> <87wpqcv4yq.fsf@linaro.org> From: Richard Henderson Message-ID: <56BB7A57.6050502@twiddle.net> Date: Thu, 11 Feb 2016 04:58:47 +1100 MIME-Version: 1.0 In-Reply-To: <87wpqcv4yq.fsf@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v5 6/9] qemu-log: new option -dfilter to limit output List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, dgilbert@redhat.com, crosthwaitepeter@gmail.com, pbonzini@redhat.com, aurelien@aurel32.net On 02/11/2016 04:40 AM, Alex Bennée wrote: > OK I think this version is a lot cleaner: > > void qemu_set_dfilter_ranges(const char *filter_spec) > { > gchar **ranges = g_strsplit(filter_spec, ",", 0); > if (ranges) { > gchar **next = ranges; > gchar *r = *next++; > debug_regions = g_array_sized_new(FALSE, FALSE, > sizeof(Range), g_strv_length(ranges)); > while (r) { > gchar *range_op = g_strstr_len(r, -1, "-"); > gchar *r2 = range_op ? range_op + 1 : NULL; > if (!range_op) { > range_op = g_strstr_len(r, -1, "+"); > r2 = range_op ? range_op + 1 : NULL; > } > if (!range_op) { > range_op = g_strstr_len(r, -1, ".."); > r2 = range_op ? range_op + 2 : NULL; > } I guess I'll quit quibbling about silly glib functions. But really, with the -1 argument, you gain nothing except obfuscation over using the standard C library. > if (range_op) { > struct Range range; > int err; > const char *e = NULL; > > err = qemu_strtoull(r, &e, 0, &range.begin); > > g_assert(e == range_op); > > switch (*range_op) { > case '+': > { > unsigned long len; > err |= qemu_strtoull(r2, NULL, 0, &len); You can't or errno's together and then... > g_error("Failed to parse range in: %s, %d", r, err); ... expect to get anything meaningful out of them. >>> + case '+': >>> + { >>> + unsigned long len; >>> + err |= qemu_strtoul(range_val, NULL, 0, &len); >>> + range.end = range.begin + len; >>> + break; >>> + } >>> + case '-': >>> + { >>> + unsigned long len; >>> + err |= qemu_strtoul(range_val, NULL, 0, &len); >>> + range.end = range.begin; >>> + range.begin = range.end - len; >>> + break; >>> + } >> >> Both of these have off-by-one bugs, since end is inclusive. > > Sorry I don't quite follow, do you mean the position of range_val (now > r2) or the final value of range.end? Final value of range.end. In that 0x1000..0x1000 and 0x1000+1 should both produce a range that covers a single byte at 0x1000. r~