qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 6/9] qemu-log: new option -dfilter to limit o


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~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]