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~