thr3ads.net - Libguestfs - [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant [Mar 2014]

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2014-Mar-19 16:53 UTC

[Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

Switch the lex/yacc parser into reentrant mode, to ease the handling of
parsing-specific data; introduce a new parser_context struct for that,
which is added as extra data to the parser.

This should cause no behaviour changes in the parsing, just no more
global variables used for getting data in/out the parser.
---
 builder/index-parse.y    | 44 ++++++++++++++++++++++++++++++++++++------
 builder/index-parser-c.c | 24 +++++++++++++----------
 builder/index-scan.l     | 50 +++++++++++++++++++++++++++++++++---------------
 builder/index-struct.c   | 14 +++++++++-----
 builder/index-struct.h   | 19 +++++++++++-------
 builder/index-validate.c | 20 +++++++++++--------
 6 files changed, 120 insertions(+), 51 deletions(-)

diff --git a/builder/index-parse.y b/builder/index-parse.y
index a8d2f62..dee2aec 100644
--- a/builder/index-parse.y
+++ b/builder/index-parse.y
@@ -24,9 +24,14 @@
 #include <string.h>
 
 #include "index-struct.h"
+#include "index-parse.h"
 
-extern void yyerror (const char *);
-extern int yylex (void);
+#define YY_EXTRA_TYPE struct parse_context *
+
+extern void yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context
*context, const char *msg);
+extern int yylex (YYSTYPE * yylval, YYLTYPE * yylloc, yyscan_t scanner);
+extern void scanner_init (yyscan_t *scanner, struct parse_context *context,
FILE *in);
+extern void scanner_destroy (yyscan_t scanner);
 
 /* Join two strings with \n */
 static char *
@@ -52,6 +57,14 @@ concat_newline (const char *str1, const char *str2)
 
 %}
 
+%code requires {
+#include "index-parse.h"
+#ifndef YY_TYPEDEF_YY_SCANNER_T
+#define YY_TYPEDEF_YY_SCANNER_T
+typedef void *yyscan_t;
+#endif
+}
+
 %locations
 
 %union {
@@ -71,13 +84,19 @@ concat_newline (const char *str1, const char *str2)
 %type <field>   fields field
 %type <str>     continuations
 
+%pure-parser
+
+%lex-param   { yyscan_t scanner }
+%parse-param { yyscan_t scanner }
+%parse-param { struct parse_context *context }
+
 %%
 
 index:
       sections
-        { parsed_index = $1; }
+        { context->parsed_index = $1; }
     | PGP_PROLOGUE sections PGP_EPILOGUE
-        { parsed_index = $2; }
+        { context->parsed_index = $2; }
 
 sections:
       section emptylines
@@ -122,8 +141,21 @@ emptylines:
 %%
 
 void
-yyerror (const char *msg)
+yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context,
const char *msg)
 {
   fprintf (stderr, "syntax error at line %d: %s\n",
-           yylloc.first_line, msg);
+           yylloc->first_line, msg);
+}
+
+int
+do_parse (struct parse_context *context, FILE *in)
+{
+  yyscan_t scanner;
+  int res;
+
+  scanner_init (&scanner, context, in);
+  res = yyparse (scanner, context);
+  scanner_destroy (scanner);
+
+  return res;
 }
diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
index fbbebff..7aeb6d0 100644
--- a/builder/index-parser-c.c
+++ b/builder/index-parser-c.c
@@ -43,7 +43,7 @@ extern void unix_error (int errcode, char * cmdname, value
arg) Noreturn;
 #include "index-struct.h"
 #include "index-parse.h"
 
-extern FILE *yyin;
+extern int do_parse (struct parse_context *context, FILE *in);
 
 value
 virt_builder_parse_index (value filenamev)
@@ -52,26 +52,30 @@ virt_builder_parse_index (value filenamev)
   CAMLlocal5 (rv, v, sv, sv2, fv);
   struct section *sections;
   size_t i, nr_sections;
+  struct parse_context context;
+  FILE *in;
 
-  yyin = fopen (String_val (filenamev), "r");
-  if (yyin == NULL)
+  parse_context_init (&context);
+
+  in = fopen (String_val (filenamev), "r");
+  if (in == NULL)
     unix_error (errno, (char *) "fopen", filenamev);
 
-  if (yyparse () != 0) {
-    fclose (yyin);
+  if (do_parse (&context, in) != 0) {
+    fclose (in);
     caml_invalid_argument ("parse error");
   }
 
-  if (fclose (yyin) == EOF)
+  if (fclose (in) == EOF)
     unix_error (errno, (char *) "fclose", filenamev);
 
   /* Convert the parsed data to OCaml structures. */
   nr_sections = 0;
-  for (sections = parsed_index; sections != NULL; sections = sections->next)
+  for (sections = context.parsed_index; sections != NULL; sections =
sections->next)
     nr_sections++;
   rv = caml_alloc (nr_sections, 0);
 
-  for (i = 0, sections = parsed_index; sections != NULL;
+  for (i = 0, sections = context.parsed_index; sections != NULL;
        i++, sections = sections->next) {
     struct field *fields;
     size_t j, nr_fields;
@@ -105,8 +109,8 @@ virt_builder_parse_index (value filenamev)
     Store_field (rv, i, v);     /* assign to return array of sections */
   }
 
-  /* Free parsed global data. */
-  free_index ();
+  /* Free parsed data. */
+  parse_context_free (&context);
 
   CAMLreturn (rv);
 }
diff --git a/builder/index-scan.l b/builder/index-scan.l
index 832ea51..073d85f 100644
--- a/builder/index-scan.l
+++ b/builder/index-scan.l
@@ -23,18 +23,22 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include "index-parse.h"
 #include "index-struct.h"
+#include "index-parse.h"
 
-#define YY_USER_ACTION yylloc.first_line = yylloc.last_line = yylineno;
+#define YY_EXTRA_TYPE struct parse_context *
+#define YY_USER_ACTION yylloc->first_line = yylloc->last_line = yylineno;
 
-extern void yyerror (const char *);
+extern void yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context
*context, const char *msg);
 
 %}
 
 %option nounput
 %option noyywrap
 %option yylineno
+%option reentrant
+%option bison-bridge
+%option bison-locations
 
 %%
 
@@ -46,38 +50,38 @@ extern void yyerror (const char *);
   */
 
   /* Ignore comments - '#' MUST appear at the start of a line. */
-^"#".*\n                { seen_comments++; }
+^"#".*\n                { yyextra->seen_comments++; }
 
   /* An empty line is significant. */
 ^\n                                     { return EMPTY_LINE; }
 
   /* [...] marks beginning of a section. */
 ^"["[-A-Za-z0-9._]+"]"\n {
-                      yylval.str = strndup (yytext+1, yyleng-3);
+                      yylval->str = strndup (yytext+1, yyleng-3);
                       return SECTION_HEADER;
                     }
 
   /* field=value or field[subfield]=value */
 ^[A-Za-z0-9_.]+("["[A-Za-z0-9_,.]+"]")?"=".*\n {
                       size_t i = strcspn (yytext, "=[");
-                      yylval.field = malloc (sizeof (struct field));
-                      yylval.field->next = NULL;
-                      yylval.field->key = strndup (yytext, i);
+                      yylval->field = malloc (sizeof (struct field));
+                      yylval->field->next = NULL;
+                      yylval->field->key = strndup (yytext, i);
                       if (yytext[i] == '[') {
                         size_t j = strcspn (yytext+i+1, "]");
-                        yylval.field->subkey = strndup (yytext+i+1, j);
+                        yylval->field->subkey = strndup (yytext+i+1, j);
                         i += 1+j+1;
                       } else {
-                        yylval.field->subkey = NULL;
+                        yylval->field->subkey = NULL;
                       }
                       /* Note we chop the final \n off here. */
-                      yylval.field->value = strndup (yytext+i+1,
yyleng-(i+2));
+                      yylval->field->value = strndup (yytext+i+1,
yyleng-(i+2));
                       return FIELD;
                     }
 
   /* Continuation line for multi-line values. */
 ^[[:blank:]].*\n        {
-                      yylval.str = strndup (yytext+1, yyleng-2);
+                      yylval->str = strndup (yytext+1, yyleng-2);
                       return VALUE_CONT;
                     }
 
@@ -86,7 +90,7 @@ extern void yyerror (const char *);
   int c, prevnl = 0;
 
   /* Eat everything to the first blank line. */
-  while ((c = input ()) != EOF) {
+  while ((c = input (yyscanner)) != EOF) {
     if (c == '\n' && prevnl)
       break;
     prevnl = c == '\n';
@@ -98,7 +102,7 @@ extern void yyerror (const char *);
  /* Hack to eat the PGP epilogue. */
 ^"-----BEGIN PGP SIGNATURE-----\n"  {
   /* Eat everything to the end of the file. */
-  while (input () != EOF)
+  while (input (yyscanner) != EOF)
     ;
 
   return PGP_EPILOGUE;
@@ -106,6 +110,22 @@ extern void yyerror (const char *);
 
  /* anything else is an error */
 . {
-  yyerror ("unexpected character in input");
+  yyerror (yylloc, yyscanner, yyextra, "unexpected character in
input");
   exit (EXIT_FAILURE);
 }
+
+%%
+
+void
+scanner_init (yyscan_t *scanner, struct parse_context *context, FILE *in)
+{
+  yylex_init (scanner);
+  yyset_extra (context, *scanner);
+  yyset_in (in, *scanner);
+}
+
+void
+scanner_destroy (yyscan_t scanner)
+{
+  yylex_destroy (scanner);
+}
diff --git a/builder/index-struct.c b/builder/index-struct.c
index fe5b0e3..f32534c 100644
--- a/builder/index-struct.c
+++ b/builder/index-struct.c
@@ -20,19 +20,23 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #include "index-struct.h"
 
-struct section *parsed_index = NULL;
-int seen_comments = 0;
-
 static void free_section (struct section *section);
 static void free_field (struct field *field);
 
 void
-free_index (void)
+parse_context_init (struct parse_context *context)
+{
+  memset (context, 0, sizeof *context);
+}
+
+void
+parse_context_free (struct parse_context *context)
 {
-  free_section (parsed_index);
+  free_section (context->parsed_index);
 }
 
 static void
diff --git a/builder/index-struct.h b/builder/index-struct.h
index f92e01d..9480526 100644
--- a/builder/index-struct.h
+++ b/builder/index-struct.h
@@ -36,14 +36,19 @@ struct field {
   char *value;
 };
 
-/* The parser (yyparse) stores the result here. */
-extern struct section *parsed_index;
+/* A struct holding the data needed during the parsing. */
+struct parse_context {
+  struct section *parsed_index;        /* The result of the parsing. */
+  /* yyparse sets this if any comments were seen.  Required for checking
+   * compatibility with virt-builder 1.24.
+   */
+  int seen_comments;
+};
 
-/* yyparse sets this if any comments were seen.  Required for checking
- * compatibility with virt-builder 1.24.
- */
-extern int seen_comments;
+/* Initialize the content of a parse_context. */
+extern void parse_context_init (struct parse_context *state);
 
-extern void free_index (void);
+/* Free the content of a parse_context.  The actual pointer is not freed. */
+extern void parse_context_free (struct parse_context *state);
 
 #endif /* INDEX_STRUCT_H */
diff --git a/builder/index-validate.c b/builder/index-validate.c
index 7f02ffb..4b7fe93 100644
--- a/builder/index-validate.c
+++ b/builder/index-validate.c
@@ -33,7 +33,7 @@
 #include "index-struct.h"
 #include "index-parse.h"
 
-extern FILE *yyin;
+extern int do_parse (struct parse_context *context, FILE *in);
 
 static void
 usage (int exit_status)
@@ -60,11 +60,15 @@ main (int argc, char *argv[])
   int compat_1_24_1 = 0;
   const char *input;
   struct section *sections;
+  struct parse_context context;
+  FILE *in;
 
   setlocale (LC_ALL, "");
   bindtextdomain (PACKAGE, LOCALEBASEDIR);
   textdomain (PACKAGE);
 
+  parse_context_init (&context);
+
   for (;;) {
     c = getopt_long (argc, argv, options, long_options, &option_index);
     if (c == -1) break;
@@ -99,32 +103,32 @@ main (int argc, char *argv[])
 
   input = argv[optind++];
 
-  yyin = fopen (input, "r");
-  if (yyin == NULL) {
+  in = fopen (input, "r");
+  if (in == NULL) {
     perror (input);
     exit (EXIT_FAILURE);
   }
 
-  if (yyparse () != 0) {
+  if (do_parse (&context, in) != 0) {
     fprintf (stderr, _("%s: '%s' could not be validated, see
errors above\n"),
              program_name, input);
     exit (EXIT_FAILURE);
   }
 
-  if (fclose (yyin) == EOF) {
+  if (fclose (in) == EOF) {
     fprintf (stderr, _("%s: %s: error reading input file: %m\n"),
              program_name, input);
     exit (EXIT_FAILURE);
   }
 
-  if (compat_1_24_1 && seen_comments) {
+  if (compat_1_24_1 && context.seen_comments) {
     fprintf (stderr, _("%s: %s contains comments which will not work with
virt-builder 1.24.1\n"),
              program_name, input);
     exit (EXIT_FAILURE);
   }
 
   /* Iterate over the parsed sections, semantically validating it. */
-  for (sections = parsed_index; sections != NULL; sections = sections->next)
{
+  for (sections = context.parsed_index; sections != NULL; sections =
sections->next) {
     int seen_sig = 0;
     struct field *fields;
 
@@ -165,7 +169,7 @@ main (int argc, char *argv[])
   }
 
   /* Free the parsed data. */
-  free_index ();
+  parse_context_free (&context);
 
   printf ("%s validated OK\n", input);
 
-- 
1.8.3.1

Pino Toscano

2014-Mar-19 16:53 UTC

head link

[Libguestfs] [PATCH 2/3] builder: show the file name in errors of the index parser

---
 builder/index-parse.y    | 4 +++-
 builder/index-parser-c.c | 1 +
 builder/index-struct.h   | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/builder/index-parse.y b/builder/index-parse.y
index dee2aec..9c75f71 100644
--- a/builder/index-parse.y
+++ b/builder/index-parse.y
@@ -143,7 +143,9 @@ emptylines:
 void
 yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context,
const char *msg)
 {
-  fprintf (stderr, "syntax error at line %d: %s\n",
+  fprintf (stderr, "%s%ssyntax error at line %d: %s\n",
+           context->input_file ? context->input_file : "",
+           context->input_file ? ": " : "",
            yylloc->first_line, msg);
 }
 
diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
index 7aeb6d0..8cae7b8 100644
--- a/builder/index-parser-c.c
+++ b/builder/index-parser-c.c
@@ -56,6 +56,7 @@ virt_builder_parse_index (value filenamev)
   FILE *in;
 
   parse_context_init (&context);
+  context.input_file = String_val (filenamev);
 
   in = fopen (String_val (filenamev), "r");
   if (in == NULL)
diff --git a/builder/index-struct.h b/builder/index-struct.h
index 9480526..7d4b8e0 100644
--- a/builder/index-struct.h
+++ b/builder/index-struct.h
@@ -43,6 +43,7 @@ struct parse_context {
    * compatibility with virt-builder 1.24.
    */
   int seen_comments;
+  const char *input_file;
 };
 
 /* Initialize the content of a parse_context. */
-- 
1.8.3.1

Pino Toscano

2014-Mar-19 16:53 UTC

head link

[Libguestfs] [PATCH 3/3] builder: show the application name in errors of the index parser

---
 builder/index-parse.y    | 4 +++-
 builder/index-parser-c.c | 5 +++--
 builder/index-struct.h   | 1 +
 builder/index_parser.ml  | 2 +-
 builder/ini_reader.ml    | 6 +++---
 builder/ini_reader.mli   | 2 +-
 builder/sources.ml       | 2 +-
 7 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/builder/index-parse.y b/builder/index-parse.y
index 9c75f71..5554e53 100644
--- a/builder/index-parse.y
+++ b/builder/index-parse.y
@@ -143,7 +143,9 @@ emptylines:
 void
 yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context,
const char *msg)
 {
-  fprintf (stderr, "%s%ssyntax error at line %d: %s\n",
+  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s\n",
+           context->program_name ? context->program_name : "",
+           context->program_name ? ": " : "",
            context->input_file ? context->input_file : "",
            context->input_file ? ": " : "",
            yylloc->first_line, msg);
diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
index 8cae7b8..5dcc82f 100644
--- a/builder/index-parser-c.c
+++ b/builder/index-parser-c.c
@@ -46,9 +46,9 @@ extern void unix_error (int errcode, char * cmdname, value
arg) Noreturn;
 extern int do_parse (struct parse_context *context, FILE *in);
 
 value
-virt_builder_parse_index (value filenamev)
+virt_builder_parse_index (value progv, value filenamev)
 {
-  CAMLparam1 (filenamev);
+  CAMLparam2 (progv, filenamev);
   CAMLlocal5 (rv, v, sv, sv2, fv);
   struct section *sections;
   size_t i, nr_sections;
@@ -56,6 +56,7 @@ virt_builder_parse_index (value filenamev)
   FILE *in;
 
   parse_context_init (&context);
+  context.program_name = String_val (progv);
   context.input_file = String_val (filenamev);
 
   in = fopen (String_val (filenamev), "r");
diff --git a/builder/index-struct.h b/builder/index-struct.h
index 7d4b8e0..7e16637 100644
--- a/builder/index-struct.h
+++ b/builder/index-struct.h
@@ -44,6 +44,7 @@ struct parse_context {
    */
   int seen_comments;
   const char *input_file;
+  const char *program_name;
 };
 
 /* Initialize the content of a parse_context. */
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index 472a6c7..5d566f9 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -119,7 +119,7 @@ let get_index ~prog ~debug ~downloader ~sigchecker source   
Sigchecker.verify sigchecker tmpfile;
 
     (* Try parsing the file. *)
-    let sections = Ini_reader.read_ini tmpfile in
+    let sections = Ini_reader.read_ini ~prog tmpfile in
     if delete_tmpfile then
       (try Unix.unlink tmpfile with _ -> ());
 
diff --git a/builder/ini_reader.ml b/builder/ini_reader.ml
index fbd4d2f..68e3863 100644
--- a/builder/ini_reader.ml
+++ b/builder/ini_reader.ml
@@ -27,10 +27,10 @@ and c_section = string * c_fields             (* [name] +
fields *)
 and c_fields = field array
 
 (* Calls yyparse in the C code. *)
-external parse_index : string -> c_sections =
"virt_builder_parse_index"
+external parse_index : prog:string -> string -> c_sections =
"virt_builder_parse_index"
 
-let read_ini file -  let sections = parse_index file in
+let read_ini ~prog file +  let sections = parse_index ~prog file in
   let sections = Array.to_list sections in
   List.map (
     fun (n, fields) ->
diff --git a/builder/ini_reader.mli b/builder/ini_reader.mli
index 992a1cb..ac3bebe 100644
--- a/builder/ini_reader.mli
+++ b/builder/ini_reader.mli
@@ -21,4 +21,4 @@ and section = string * fields                (* [name] +
fields *)
 and fields = field list
 and field = string * string option * string  (* key + subkey + value *)
 
-val read_ini : string -> sections
+val read_ini : prog:string -> string -> sections
diff --git a/builder/sources.ml b/builder/sources.ml
index 016adc4..1fee65e 100644
--- a/builder/sources.ml
+++ b/builder/sources.ml
@@ -32,7 +32,7 @@ let parse_conf ~prog ~debug file    if debug then (
     eprintf (f_"%s: trying to read %s\n") prog file;
   );
-  let sections = Ini_reader.read_ini file in
+  let sections = Ini_reader.read_ini ~prog file in
 
   let sources = List.fold_right (
     fun (n, fields) acc ->
-- 
1.8.3.1

Richard W.M. Jones

2014-Mar-19 22:15 UTC

head link

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

On Wed, Mar 19, 2014 at 05:53:09PM +0100, Pino Toscano
wrote:> Switch the lex/yacc parser into reentrant mode, to ease the handling of
> parsing-specific data; introduce a new parser_context struct for that,
> which is added as extra data to the parser.
> 
> This should cause no behaviour changes in the parsing, just no more
> global variables used for getting data in/out the parser.
This looks like a straightforward conversion to a reentrant
parser, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)

Richard W.M. Jones

2014-Mar-19 22:16 UTC

head link

Re: [Libguestfs] [PATCH 2/3] builder: show the file name in errors of the index parser

On Wed, Mar 19, 2014 at 05:53:10PM +0100, Pino Toscano
wrote:> ---
>  builder/index-parse.y    | 4 +++-
>  builder/index-parser-c.c | 1 +
>  builder/index-struct.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/builder/index-parse.y b/builder/index-parse.y
> index dee2aec..9c75f71 100644
> --- a/builder/index-parse.y
> +++ b/builder/index-parse.y
> @@ -143,7 +143,9 @@ emptylines:
>  void
>  yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context
*context, const char *msg)
>  {
> -  fprintf (stderr, "syntax error at line %d: %s\n",
> +  fprintf (stderr, "%s%ssyntax error at line %d: %s\n",
> +           context->input_file ? context->input_file : "",
> +           context->input_file ? ": " : "",
>             yylloc->first_line, msg);
>  }
>  
> diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
> index 7aeb6d0..8cae7b8 100644
> --- a/builder/index-parser-c.c
> +++ b/builder/index-parser-c.c
> @@ -56,6 +56,7 @@ virt_builder_parse_index (value filenamev)
>    FILE *in;
>  
>    parse_context_init (&context);
> +  context.input_file = String_val (filenamev);
>  
>    in = fopen (String_val (filenamev), "r");
>    if (in == NULL)
> diff --git a/builder/index-struct.h b/builder/index-struct.h
> index 9480526..7d4b8e0 100644
> --- a/builder/index-struct.h
> +++ b/builder/index-struct.h
> @@ -43,6 +43,7 @@ struct parse_context {
>     * compatibility with virt-builder 1.24.
>     */
>    int seen_comments;
> +  const char *input_file;
>  };
>  
>  /* Initialize the content of a parse_context. */
> -- 
> 1.8.3.1
ACK.  When you commit this, can you add (RHBZ#xxxxx) to the subject
line?  It will ensure the bugs-in-changelog.sh script picks up the bug
for the release notes.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Richard W.M. Jones

2014-Mar-19 22:17 UTC

head link

Re: [Libguestfs] [PATCH 3/3] builder: show the application name in errors of the index parser

On Wed, Mar 19, 2014 at 05:53:11PM +0100, Pino Toscano
wrote:> ---
>  builder/index-parse.y    | 4 +++-
>  builder/index-parser-c.c | 5 +++--
>  builder/index-struct.h   | 1 +
>  builder/index_parser.ml  | 2 +-
>  builder/ini_reader.ml    | 6 +++---
>  builder/ini_reader.mli   | 2 +-
>  builder/sources.ml       | 2 +-
>  7 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/builder/index-parse.y b/builder/index-parse.y
> index 9c75f71..5554e53 100644
> --- a/builder/index-parse.y
> +++ b/builder/index-parse.y
> @@ -143,7 +143,9 @@ emptylines:
>  void
>  yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context
*context, const char *msg)
>  {
> -  fprintf (stderr, "%s%ssyntax error at line %d: %s\n",
> +  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s\n",
> +           context->program_name ? context->program_name :
"",
> +           context->program_name ? ": " : "",
>             context->input_file ? context->input_file : "",
>             context->input_file ? ": " : "",
>             yylloc->first_line, msg);
> diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
> index 8cae7b8..5dcc82f 100644
> --- a/builder/index-parser-c.c
> +++ b/builder/index-parser-c.c
> @@ -46,9 +46,9 @@ extern void unix_error (int errcode, char * cmdname,
value arg) Noreturn;
>  extern int do_parse (struct parse_context *context, FILE *in);
>  
>  value
> -virt_builder_parse_index (value filenamev)
> +virt_builder_parse_index (value progv, value filenamev)
>  {
> -  CAMLparam1 (filenamev);
> +  CAMLparam2 (progv, filenamev);
>    CAMLlocal5 (rv, v, sv, sv2, fv);
>    struct section *sections;
>    size_t i, nr_sections;
> @@ -56,6 +56,7 @@ virt_builder_parse_index (value filenamev)
>    FILE *in;
>  
>    parse_context_init (&context);
> +  context.program_name = String_val (progv);
>    context.input_file = String_val (filenamev);
>  
>    in = fopen (String_val (filenamev), "r");
> diff --git a/builder/index-struct.h b/builder/index-struct.h
> index 7d4b8e0..7e16637 100644
> --- a/builder/index-struct.h
> +++ b/builder/index-struct.h
> @@ -44,6 +44,7 @@ struct parse_context {
>     */
>    int seen_comments;
>    const char *input_file;
> +  const char *program_name;
>  };
>  
>  /* Initialize the content of a parse_context. */
> diff --git a/builder/index_parser.ml b/builder/index_parser.ml
> index 472a6c7..5d566f9 100644
> --- a/builder/index_parser.ml
> +++ b/builder/index_parser.ml
> @@ -119,7 +119,7 @@ let get_index ~prog ~debug ~downloader ~sigchecker
source >      Sigchecker.verify sigchecker tmpfile;
>  
>      (* Try parsing the file. *)
> -    let sections = Ini_reader.read_ini tmpfile in
> +    let sections = Ini_reader.read_ini ~prog tmpfile in
>      if delete_tmpfile then
>        (try Unix.unlink tmpfile with _ -> ());
>  
> diff --git a/builder/ini_reader.ml b/builder/ini_reader.ml
> index fbd4d2f..68e3863 100644
> --- a/builder/ini_reader.ml
> +++ b/builder/ini_reader.ml
> @@ -27,10 +27,10 @@ and c_section = string * c_fields             (* [name]
+ fields *)
>  and c_fields = field array
>  
>  (* Calls yyparse in the C code. *)
> -external parse_index : string -> c_sections =
"virt_builder_parse_index"
> +external parse_index : prog:string -> string -> c_sections =
"virt_builder_parse_index"
>  
> -let read_ini file > -  let sections = parse_index file in
> +let read_ini ~prog file > +  let sections = parse_index ~prog file in
>    let sections = Array.to_list sections in
>    List.map (
>      fun (n, fields) ->
> diff --git a/builder/ini_reader.mli b/builder/ini_reader.mli
> index 992a1cb..ac3bebe 100644
> --- a/builder/ini_reader.mli
> +++ b/builder/ini_reader.mli
> @@ -21,4 +21,4 @@ and section = string * fields                (* [name] +
fields *)
>  and fields = field list
>  and field = string * string option * string  (* key + subkey + value *)
>  
> -val read_ini : string -> sections
> +val read_ini : prog:string -> string -> sections
> diff --git a/builder/sources.ml b/builder/sources.ml
> index 016adc4..1fee65e 100644
> --- a/builder/sources.ml
> +++ b/builder/sources.ml
> @@ -32,7 +32,7 @@ let parse_conf ~prog ~debug file >    if debug then (
>      eprintf (f_"%s: trying to read %s\n") prog file;
>    );
> -  let sections = Ini_reader.read_ini file in
> +  let sections = Ini_reader.read_ini ~prog file in
>  
>    let sources = List.fold_right (
>      fun (n, fields) acc ->
ACK.

Thanks, Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Richard W.M. Jones

2014-Mar-29 12:31 UTC

head link

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

On Wed, Mar 19, 2014 at 05:53:09PM +0100, Pino Toscano
wrote:> Switch the lex/yacc parser into reentrant mode, to ease the handling of
> parsing-specific data; introduce a new parser_context struct for that,
> which is added as extra data to the parser.
> 
> This should cause no behaviour changes in the parsing, just no more
> global variables used for getting data in/out the parser.
> +%code requires {
> +#include "index-parse.h"
There's a strange problem here with old Bison in Gentoo:

  CC       virt_index_validate-index-parse.o
In file included from index-parse.y:61:0,
                 from index-parse.y:61,
                 from index-parse.y:61,
                 from index-parse.y:61,
                 from index-parse.y:61,
                 from index-parse.y:61,
[etc]

Basically index-parse.h includes itself.

This seems to be fixed somewhere between Bison 2.4.3 and Bison 2.7.
In 2.7, Bison adds a #ifndef/#define to prevent recursive includes.

RHEL 6 bison 2.4.1 is < 2.4.3, but for reasons I don't quite
understand doesn't appear to be affected.

Not sure if we need to fix this, just noting it's a potential problem.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Richard W.M. Jones

2014-Mar-29 15:11 UTC

head link

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

On Sat, Mar 29, 2014 at 12:31:24PM +0000, Richard W.M. Jones
wrote:> On Wed, Mar 19, 2014 at 05:53:09PM +0100, Pino Toscano wrote:
> > Switch the lex/yacc parser into reentrant mode, to ease the handling
of
> > parsing-specific data; introduce a new parser_context struct for that,
> > which is added as extra data to the parser.
> > 
> > This should cause no behaviour changes in the parsing, just no more
> > global variables used for getting data in/out the parser.
> > +%code requires {
> > +#include "index-parse.h"
> 
> There's a strange problem here with old Bison in Gentoo:
> 
>   CC       virt_index_validate-index-parse.o
> In file included from index-parse.y:61:0,
>                  from index-parse.y:61,
>                  from index-parse.y:61,
>                  from index-parse.y:61,
>                  from index-parse.y:61,
>                  from index-parse.y:61,
> [etc]
> 
> Basically index-parse.h includes itself.
> 
> This seems to be fixed somewhere between Bison 2.4.3 and Bison 2.7.
> In 2.7, Bison adds a #ifndef/#define to prevent recursive includes.
> 
> RHEL 6 bison 2.4.1 is < 2.4.3, but for reasons I don't quite
> understand doesn't appear to be affected.
Turns out it *does* affect RHEL 6.  I didn't see the problem because I
didn't sufficiently 'make clean'.

I pushed this which fixes it for me on RHEL 6 & Fedora 20:

https://github.com/libguestfs/libguestfs/commit/eff7aed6cfc24c1d605578f738a0d76112d9966b

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Mar 2014 - [PATCH 1/3] builder: make the C index parser reentrant

[Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

[Libguestfs] [PATCH 2/3] builder: show the file name in errors of the index parser

[Libguestfs] [PATCH 3/3] builder: show the application name in errors of the index parser

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

Re: [Libguestfs] [PATCH 2/3] builder: show the file name in errors of the index parser

Re: [Libguestfs] [PATCH 3/3] builder: show the application name in errors of the index parser

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

Re: [Libguestfs] [PATCH 1/3] builder: make the C index parser reentrant

Possibly Parallel Threads