[glib/parser] parser tests and fixes



commit 13b8797ecdf40c10dd602426323b2c01b833290d
Author: Ryan Lortie <desrt desrt ca>
Date:   Fri Mar 19 17:04:33 2010 -0500

    parser tests and fixes

 glib/gvariant-parser.c |   91 +++++++++++++++---------
 glib/gvariant.c        |    6 ++-
 glib/tests/gvariant.c  |  186 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 248 insertions(+), 35 deletions(-)
---
diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c
index 767434d..b1c07b6 100644
--- a/glib/gvariant-parser.c
+++ b/glib/gvariant-parser.c
@@ -57,9 +57,16 @@ parser_set_error_va (GError      **error,
 {
   GString *msg = g_string_new (NULL);
 
-  g_string_append_printf (msg, "%d-%d", location->start, location->end);
+  if (location->start == location->end)
+    g_string_append_printf (msg, "%d", location->start);
+  else
+    g_string_append_printf (msg, "%d-%d", location->start, location->end);
+
   if (other != NULL)
-    g_string_append_printf (msg, ",%d-%d", other->start, other->end);
+    {
+      g_assert (other->start != other->end);
+      g_string_append_printf (msg, ",%d-%d", other->start, other->end);
+    }
   g_string_append_c (msg, ':');
 
   g_string_append_vprintf (msg, format, ap);
@@ -153,11 +160,11 @@ token_stream_prepare (TokenStream *stream)
       break;
 
     case '@': case '%':
-      /* stop at the first space or unmatched bracket.
+      /* stop at the first space, comma or unmatched bracket.
        * deals nicely with cases like (%i, %i).
        */
       for (end = stream->stream + 1;
-           end != stream->end && !g_ascii_isspace (*end);
+           end != stream->end && *end != ',' && !g_ascii_isspace (*end);
            end++)
 
         if (*end == '(' || *end == '{')
@@ -589,13 +596,16 @@ ast_array_get_pattern (AST    **array,
          * pair of values.
          */
         {
-          int j;
+          int j = 0;
 
-          for (j = 0; j < i; j++)
+          while (TRUE)
             {
               gchar *tmp2;
               gchar *m;
 
+              /* if 'j' reaches 'i' then we failed to find the pair */
+              g_assert (j < i);
+
               tmp2 = ast_get_pattern (array[j], NULL);
               g_assert (tmp2 != NULL);
 
@@ -614,10 +624,9 @@ ast_array_get_pattern (AST    **array,
                   g_free (tmp);
                   return NULL;
                 }
-            }
 
-          /* should have found it... */
-          g_assert_not_reached ();
+              j++;
+            }
         }
     }
 
@@ -643,6 +652,10 @@ maybe_get_pattern (AST     *ast,
       gchar *pattern;
 
       child_pattern = ast_get_pattern (maybe->child, error);
+
+      if (child_pattern == NULL)
+        return NULL;
+
       pattern = g_strdup_printf ("m%s", child_pattern);
       g_free (child_pattern);
 
@@ -786,6 +799,9 @@ array_get_value (AST                 *ast,
   GVariantBuilder builder;
   gint i;
 
+  if (!g_variant_type_is_array (type))
+    return ast_type_error (ast, type, error);
+
   g_variant_builder_init (&builder, type);
   childtype = g_variant_type_element (type);
 
@@ -839,7 +855,7 @@ array_parse (TokenStream  *stream,
 
       if (need_comma &&
           !token_stream_require (stream, ",",
-                                 " or ']' to follow array element",
+                                 " or `]' to follow array element",
                                  error))
         goto error;
 
@@ -908,6 +924,9 @@ tuple_get_value (AST                 *ast,
   GVariantBuilder builder;
   gint i;
 
+  if (!g_variant_type_is_tuple (type))
+    return ast_type_error (ast, type, error);
+
   g_variant_builder_init (&builder, type);
   childtype = g_variant_type_first (type);
 
@@ -963,7 +982,7 @@ tuple_parse (TokenStream  *stream,
 
       if (need_comma &&
           !token_stream_require (stream, ",",
-                                 " or ')' to follow tuple element",
+                                 " or `)' to follow tuple element",
                                  error))
         goto error;
 
@@ -1146,6 +1165,9 @@ dictionary_get_value (AST                 *ast,
       GVariantBuilder builder;
       GVariant *subvalue;
 
+      if (!g_variant_type_is_dict_entry (type))
+        return ast_type_error (ast, type, error);
+
       g_variant_builder_init (&builder, type);
 
       subtype = g_variant_type_key (type);
@@ -1172,6 +1194,9 @@ dictionary_get_value (AST                 *ast,
       GVariantBuilder builder;
       gint i;
 
+      if (!g_variant_type_is_subtype_of (type, G_VARIANT_TYPE_DICTIONARY))
+        return ast_type_error (ast, type, error);
+
       entry = g_variant_type_element (type);
       key = g_variant_type_key (entry);
       val = g_variant_type_value (entry);
@@ -1257,7 +1282,7 @@ dictionary_parse (TokenStream  *stream,
   only_one = token_stream_consume (stream, ",");
   if (!only_one &&
       !token_stream_require (stream, ":",
-                             " or ',' to follow dictionary entry key",
+                             " or `,' to follow dictionary entry key",
                              error))
     goto error;
 
@@ -1283,7 +1308,7 @@ dictionary_parse (TokenStream  *stream,
       AST *child;
 
       if (!token_stream_require (stream, ",",
-                                 " or '}' to follow dictionary entry", error))
+                                 " or `}' to follow dictionary entry", error))
         goto error;
 
       child = parse (stream, app, error);
@@ -1487,7 +1512,7 @@ number_overflow (AST                 *ast,
                  const GVariantType  *type,
                  GError             **error)
 {
-  ast_set_error (ast, error, NULL, "number too big for type `%c'",
+  ast_set_error (ast, error, NULL, "number out of range for type `%c'",
                  g_variant_type_peek_string (type)[0]);
   return NULL;
 }
@@ -1543,7 +1568,7 @@ number_get_value (AST                 *ast,
       SourceRef ref;
 
       ref = ast->source_ref;
-      ref.start += token - number->token;
+      ref.start += end - number->token;
       ref.end = ref.start + 1;
 
       parser_set_error (error, &ref, NULL,
@@ -1725,8 +1750,9 @@ positional_free (AST *ast)
 {
   Positional *positional = (Positional *) ast;
 
-  if (positional->value)
-    g_variant_unref (g_variant_ref_sink (positional->value));
+  /* if positional->value is set, just leave it.
+   * memory management doesn't matter in case of programmer error.
+   */
   g_slice_free (Positional, positional);
 }
 
@@ -1755,9 +1781,7 @@ positional_parse (TokenStream  *stream,
     {
       token_stream_set_error (stream, error, TRUE,
                               "invalid GVariant format string");
-      /* memory management doesn't matter.
-       * we're going to have a g_error() quite soon...
-       */
+      /* memory management doesn't matter in case of programmer error. */
       return NULL;
     }
 
@@ -2047,8 +2071,11 @@ g_variant_parse (const GVariantType  *type,
 
               if (stream.stream != limit && *stream.stream != '\0')
                 {
-                  token_stream_set_error (&stream, error, FALSE,
-                                          "expected end of input");
+                  SourceRef ref = { stream.stream - text,
+                                    stream.stream - text };
+
+                  parser_set_error (error, &ref, NULL,
+                                    "expected end of input");
                   g_variant_unref (result);
 
                   result = NULL;
@@ -2097,18 +2124,11 @@ g_variant_new_parsed_va (const gchar *format,
       ast_free (ast);
     }
 
-  if (result != NULL)
-    {
-      while (g_ascii_isspace (*stream.stream))
-        stream.stream++;
+  if (result == NULL)
+    g_error ("g_variant_new_parsed: %s", error->message);
 
-      if (*stream.stream)
-        g_error ("g_variant_new_parsed: trailing text after value");
-    }
-  else
-    {
-      g_error ("g_variant_new_parsed: %s", error->message);
-    }
+  if (*stream.stream)
+    g_error ("g_variant_new_parsed: trailing text after value");
 
   return result;
 }
@@ -2139,6 +2159,11 @@ g_variant_new_parsed_va (const gchar *format,
  * This function is intended only to be used with @format as a string
  * literal.  Any parse error is fatal to the calling process.  If you
  * want to parse data from untrusted sources, use g_variant_parse().
+ *
+ * You may not use this function to return, unmodified, a single
+ * #GVariant pointer from the argument list.  ie: @format may not solely
+ * be anything along the lines of "%*", "%?", "%r", or anything starting
+ * with "%@".
  **/
 GVariant *
 g_variant_new_parsed (const gchar *format,
diff --git a/glib/gvariant.c b/glib/gvariant.c
index b6acfb1..c971cfc 100644
--- a/glib/gvariant.c
+++ b/glib/gvariant.c
@@ -1629,7 +1629,11 @@ g_variant_print_string (GVariant *value,
         const gchar *str = g_variant_get_string (value, NULL);
         gchar *escaped = g_strescape (str, NULL);
 
-        g_string_append_printf (string, "\'%s\'", escaped);
+        /* use double quotes only if a ' is in the string */
+        if (strchr (str, '\''))
+          g_string_append_printf (string, "\"%s\"", escaped);
+        else
+          g_string_append_printf (string, "'%s'", escaped);
 
         g_free (escaped);
       }
diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c
index 474cb12..7ece02d 100644
--- a/glib/tests/gvariant.c
+++ b/glib/tests/gvariant.c
@@ -3489,7 +3489,6 @@ test_parser (void)
   g_free (p);
 }
 
-
 static void
 test_parses (void)
 {
@@ -3500,9 +3499,192 @@ test_parses (void)
       test_parser ();
     }
 
+  /* mini test */
+  {
+    gchar str[256];
+    GVariant *val;
+    gchar *p, *p2;
+
+    for (i = 0; i < 256; i++)
+      str[i] = i + 1;
+
+    val = g_variant_new_string (str);
+    p = g_variant_print (val, FALSE);
+    g_variant_unref (val);
+
+    val = g_variant_parse (NULL, p, NULL, NULL, NULL);
+    p2 = g_variant_print (val, FALSE);
+
+    g_assert_cmpstr (str, ==, g_variant_get_string (val, NULL));
+    g_assert_cmpstr (p, ==, p2);
+
+    g_variant_unref (val);
+    g_free (p2);
+    g_free (p);
+  }
+
+  /* another mini test */
+  {
+    const gchar *end;
+    GVariant *value;
+
+    value = g_variant_parse (G_VARIANT_TYPE_INT32, "1 2 3", NULL, &end, NULL);
+    g_assert_cmpint (g_variant_get_int32 (value), ==, 1);
+    /* make sure endptr returning works */
+    g_assert_cmpstr (end, ==, " 2 3");
+    g_variant_unref (value);
+  }
+
   g_variant_type_info_assert_no_infos ();
 }
 
+static void
+test_parse_failures (void)
+{
+  const gchar *test[] = {
+    "[1, 2,",                   "6:",              "expected value",
+    "",                         "0:",              "expected value",
+    "(1, 2,",                   "6:",              "expected value",
+    "<1",                       "2:",              "expected `>'",
+    "[]",                       "0-2:",            "unable to infer",
+    "(,",                       "1:",              "expected value",
+    "[4,'']",                   "1-2,3-5:",        "common type",
+    "[4, '', 5]",               "1-2,4-6:",        "common type",
+    "['', 4, 5]",               "1-3,5-6:",        "common type",
+    "[4, 5, '']",               "1-2,7-9:",        "common type",
+    "[[4], [], ['']]",          "1-4,10-14:",      "common type",
+    "[[], [4], ['']]",          "5-8,10-14:",      "common type",
+    "just",                     "4:",              "expected value",
+    "nothing",                  "0-7:",            "unable to infer",
+    "just [4, '']",             "6-7,9-11:",       "common type",
+    "[[4,'']]",                 "2-3,4-6:",        "common type",
+    "([4,''],)",                "2-3,4-6:",        "common type",
+    "(4)",                      "2:",              "`,'",
+    "{}",                       "0-2:",            "unable to infer",
+    "{[1,2],[3,4]}",            "0-13:",           "basic types",
+    "{[1,2]:[3,4]}",            "0-13:",           "basic types",
+    "justt",                    "0-5:",            "unknown keyword",
+    "nothng",                   "0-6:",            "unknown keyword",
+    "uint33",                   "0-6:",            "unknown keyword",
+    "@mi just ''",              "9-11:",           "can not parse as",
+    "@ai ['']",                 "5-7:",            "can not parse as",
+    "@(i) ('',)",               "6-8:",            "can not parse as",
+    "[[], 5]",                  "1-3,5-6:",        "common type",
+    "[[5], 5]",                 "1-4,6-7:",        "common type",
+    "5 5",                      "2:",              "expected end of input",
+    "[5, [5, '']]",             "5-6,8-10:",       "common type",
+    "@i just 5",                "3-9:",            "can not parse as",
+    "@i nothing",               "3-10:",           "can not parse as",
+    "@i []",                    "3-5:",            "can not parse as",
+    "@i ()",                    "3-5:",            "can not parse as",
+    "@ai (4,)",                 "4-8:",            "can not parse as",
+    "@(i) []",                  "5-7:",            "can not parse as",
+    "(5 5)",                    "3:",              "expected `,'",
+    "[5 5]",                    "3:",              "expected `,' or `]'",
+    "(5, 5 5)",                 "6:",              "expected `,' or `)'",
+    "[5, 5 5]",                 "6:",              "expected `,' or `]'",
+    "<@i []>",                  "4-6:",            "can not parse as",
+    "<[5 5]>",                  "4:",              "expected `,' or `]'",
+    "{[4,''],5}",               "2-3,4-6:",        "common type",
+    "{5,[4,'']}",               "4-5,6-8:",        "common type",
+    "@i {1,2}",                 "3-8:",            "can not parse as",
+    "{ i '', 5}",               "4-6:",            "can not parse as",
+    "{5, @i ''}",               "7-9:",            "can not parse as",
+    "@ai {}",                   "4-6:",            "can not parse as",
+    "{ i '': 5}",               "4-6:",            "can not parse as",
+    "{5: @i ''}",               "7-9:",            "can not parse as",
+    "{<4,5}",                   "3:",              "expected `>'",
+    "{4,<5}",                   "5:",              "expected `>'",
+    "{4,5,6}",                  "4:",              "expected `}'",
+    "{5 5}",                    "3:",              "expected `:' or `,'",
+    "{4: 5: 6}",                "5:",              "expected `,' or `}'",
+    "{4:5,<6:7}",               "7:",              "expected `>'",
+    "{4:5,6:<7}",               "9:",              "expected `>'",
+    "{4:5,6 7}",                "7:",              "expected `:'",
+    "@o 'foo'",                 "3-8:",            "object path",
+    "@g 'zzz'",                 "3-8:",            "signature",
+    "@i true",                  "3-7:",            "can not parse as",
+    "@z 4",                     "0-2:",            "invalid type",
+    "@a* []",                   "0-3:",            "definite",
+    "@ai [3 3]",                "7:",              "expected `,' or `]'",
+    "18446744073709551616",     "0-20:",           "too big for any type",
+    "-18446744073709551616",    "0-21:",           "too big for any type",
+    "byte 256",                 "5-8:",            "out of range for type",
+    "byte -1",                  "5-7:",            "out of range for type",
+    "int16 32768",              "6-11:",           "out of range for type",
+    "int16 -32769",             "6-12:",           "out of range for type",
+    "uint16 -1",                "7-9:",            "out of range for type",
+    "uint16 65536",             "7-12:",           "out of range for type",
+    "2147483648",               "0-10:",           "out of range for type",
+    "-2147483649",              "0-11:",           "out of range for type",
+    "uint32 -1",                "7-9:",            "out of range for type",
+    "uint32 4294967296",        "7-17:",           "out of range for type",
+    "@x 9223372036854775808",   "3-22:",           "out of range for type",
+    "@x -9223372036854775809",  "3-23:",           "out of range for type",
+    "@t -1",                    "3-5:",            "out of range for type",
+    "@t 18446744073709551616",  "3-23:",           "too big for any type",
+    "handle 2147483648",        "7-17:",           "out of range for type",
+    "handle -2147483649",       "7-18:",           "out of range for type",
+    "1.798e308",                "0-9:",            "too big for any type",
+    "37.5a488",                 "4-5:",            "invalid character",
+    "0x7ffgf",                  "5-6:",            "invalid character",
+    "07758",                    "4-5:",            "invalid character",
+    "123a5",                    "3-4:",            "invalid character",
+    "@ai 123",                  "4-7:",            "can not parse as",
+    "'\"\\'",                   "0-4:",            "unterminated string",
+    "'\"\\'\\",                 "0-5:",            "unterminated string",
+  };
+  gint i;
+
+  for (i = 0; i < G_N_ELEMENTS (test); i += 3)
+    {
+      GError *error = NULL;
+      GVariant *value;
+
+      value = g_variant_parse (NULL, test[i], NULL, NULL, &error);
+      g_assert (value == NULL);
+
+      if (!strstr (error->message, test[i+2]))
+        g_error ("test %d: Can't find `%s' in `%s'", i / 3,
+                 test[i+2], error->message);
+
+      if (!g_str_has_prefix (error->message, test[i+1]))
+        g_error ("test %d: Expected location `%s' in `%s'", i / 3,
+                 test[i+1], error->message);
+
+      g_error_free (error);
+    }
+}
+
+static void
+test_parse_positional (void)
+{
+  check_and_free (g_variant_new_parsed ("[('one', 1), (%s, 2),"
+                                        " ('three', %i)]", "two", 3),
+                  "[('one', 1), ('two', 2), ('three', 3)]");
+  check_and_free (g_variant_new_parsed ("[('one', 1), (%s, 2),"
+                                        " ('three', %u)]", "two", 3),
+                  "[('one', uint32 1), ('two', 2), ('three', 3)]");
+
+  if (do_failed_test ("*invalid GVariant format*"))
+    {
+      g_variant_new_parsed ("%z");
+      abort ();
+    }
+
+  if (do_failed_test ("*incorrect type*"))
+    {
+      g_variant_new_parsed ("uint32 %i", 2);
+      abort ();
+    }
+
+  if (do_failed_test ("*incorrect type*"))
+    {
+      g_variant_new_parsed ("% i", g_variant_new_uint32 (2));
+      abort ();
+    }
+}
+
 int
 main (int argc, char **argv)
 {
@@ -3538,6 +3720,8 @@ main (int argc, char **argv)
   g_test_add_func ("/gvariant/hashing", test_hashing);
   g_test_add_func ("/gvariant/byteswap", test_gv_byteswap);
   g_test_add_func ("/gvariant/parser", test_parses);
+  g_test_add_func ("/gvariant/parse-failures", test_parse_failures);
+  g_test_add_func ("/gvariant/parse-positional", test_parse_positional);
 
   return g_test_run ();
 }



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