|
From: | Richard Henderson |
Subject: | Re: [Qemu-devel] [PATCH v5 6/9] qemu-log: new option -dfilter to limit output |
Date: | Thu, 11 Feb 2016 04:58:47 +1100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
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~
[Prev in Thread] | Current Thread | [Next in Thread] |