Libguestfs - Apr 2014 - [PATCH 1/2] builder: add an optional suffix string for INI parsing errors

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

diff --git a/builder/index-parse.y b/builder/index-parse.y
index 310870d..7ddef53 100644
--- a/builder/index-parse.y
+++ b/builder/index-parse.y
@@ -150,12 +150,14 @@ emptylines:
 void
 yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context,
const char *msg)
 {
-  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s\n",
+  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s%s%s\n",
            context->program_name ? context->program_name : "",
            context->program_name ? ": " : "",
            context->input_file ? context->input_file : "",
            context->input_file ? ": " : "",
-           yylloc->first_line, msg);
+           yylloc->first_line, msg,
+           context->error_suffix ? " " : "",
+           context->error_suffix ? context->error_suffix : "");
 }
 
 int
diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
index 5dcc82f..099bdf8 100644
--- a/builder/index-parser-c.c
+++ b/builder/index-parser-c.c
@@ -46,7 +46,7 @@ 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 progv, value filenamev)
+virt_builder_parse_index (value progv, value error_suffixv, value filenamev)
 {
   CAMLparam2 (progv, filenamev);
   CAMLlocal5 (rv, v, sv, sv2, fv);
@@ -58,6 +58,7 @@ virt_builder_parse_index (value progv, value filenamev)
   parse_context_init (&context);
   context.program_name = String_val (progv);
   context.input_file = String_val (filenamev);
+  context.error_suffix = String_val (error_suffixv);
 
   in = fopen (String_val (filenamev), "r");
   if (in == NULL)
diff --git a/builder/index-struct.h b/builder/index-struct.h
index 3edd06d..150535d 100644
--- a/builder/index-struct.h
+++ b/builder/index-struct.h
@@ -45,6 +45,7 @@ struct parse_context {
   int seen_comments;
   const char *input_file;
   const char *program_name;
+  const char *error_suffix;
 };
 
 /* Initialize the content of a parse_context. */
diff --git a/builder/ini_reader.ml b/builder/ini_reader.ml
index 68e3863..c64125c 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 : prog:string -> string -> c_sections =
"virt_builder_parse_index"
+external parse_index : prog:string -> error_suffix:string -> string ->
c_sections = "virt_builder_parse_index"
 
-let read_ini ~prog file -  let sections = parse_index ~prog file in
+let read_ini ~prog ?(error_suffix = "") file +  let sections =
parse_index ~prog ~error_suffix 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 ac3bebe..1b8e894 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 : prog:string -> string -> sections
+val read_ini : prog:string -> ?error_suffix:string -> string ->
sections
-- 
1.9.0

Parsing sources .conf files is not a fatal error (that file would just
be ignored), so explicitly state that such parsing errors are ignored.

This should address the last bit in RHBZ#1077817.
---
 builder/sources.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builder/sources.ml b/builder/sources.ml
index e7644a2..be1c27d 100644
--- a/builder/sources.ml
+++ b/builder/sources.ml
@@ -35,7 +35,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 ~prog file in
+  let sections = Ini_reader.read_ini ~prog ~error_suffix:"[ignored]"
file in
 
   let sources = List.fold_right (
     fun (n, fields) acc ->
-- 
1.9.0

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

diff --git a/builder/index-parse.y b/builder/index-parse.y
index 310870d..06e6a83 100644
--- a/builder/index-parse.y
+++ b/builder/index-parse.y
@@ -150,12 +150,16 @@ emptylines:
 void
 yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context,
const char *msg)
 {
-  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s\n",
+  int has_suffix = context->error_suffix != NULL &&
context->error_suffix[0] != 0;
+
+  fprintf (stderr, "%s%s%s%ssyntax error at line %d: %s%s%s\n",
            context->program_name ? context->program_name : "",
            context->program_name ? ": " : "",
            context->input_file ? context->input_file : "",
            context->input_file ? ": " : "",
-           yylloc->first_line, msg);
+           yylloc->first_line, msg,
+           has_suffix ? " " : "",
+           has_suffix ? context->error_suffix : "");
 }
 
 int
diff --git a/builder/index-parser-c.c b/builder/index-parser-c.c
index 5dcc82f..099bdf8 100644
--- a/builder/index-parser-c.c
+++ b/builder/index-parser-c.c
@@ -46,7 +46,7 @@ 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 progv, value filenamev)
+virt_builder_parse_index (value progv, value error_suffixv, value filenamev)
 {
   CAMLparam2 (progv, filenamev);
   CAMLlocal5 (rv, v, sv, sv2, fv);
@@ -58,6 +58,7 @@ virt_builder_parse_index (value progv, value filenamev)
   parse_context_init (&context);
   context.program_name = String_val (progv);
   context.input_file = String_val (filenamev);
+  context.error_suffix = String_val (error_suffixv);
 
   in = fopen (String_val (filenamev), "r");
   if (in == NULL)
diff --git a/builder/index-struct.h b/builder/index-struct.h
index 3edd06d..150535d 100644
--- a/builder/index-struct.h
+++ b/builder/index-struct.h
@@ -45,6 +45,7 @@ struct parse_context {
   int seen_comments;
   const char *input_file;
   const char *program_name;
+  const char *error_suffix;
 };
 
 /* Initialize the content of a parse_context. */
diff --git a/builder/ini_reader.ml b/builder/ini_reader.ml
index 68e3863..c64125c 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 : prog:string -> string -> c_sections =
"virt_builder_parse_index"
+external parse_index : prog:string -> error_suffix:string -> string ->
c_sections = "virt_builder_parse_index"
 
-let read_ini ~prog file -  let sections = parse_index ~prog file in
+let read_ini ~prog ?(error_suffix = "") file +  let sections =
parse_index ~prog ~error_suffix 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 ac3bebe..1b8e894 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 : prog:string -> string -> sections
+val read_ini : prog:string -> ?error_suffix:string -> string ->
sections
-- 
1.9.0

On Tue, Apr 22, 2014 at 05:41:44PM +0200, Pino Toscano
wrote:
> Parsing sources .conf files is not a fatal error (that file would just
> be ignored), so explicitly state that such parsing errors are ignored.
> 
> This should address the last bit in RHBZ#1077817.
> ---
>  builder/sources.ml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builder/sources.ml b/builder/sources.ml
> index e7644a2..be1c27d 100644
> --- a/builder/sources.ml
> +++ b/builder/sources.ml
> @@ -35,7 +35,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 ~prog file in
> +  let sections = Ini_reader.read_ini ~prog
~error_suffix:"[ignored]" file in
>  
>    let sources = List.fold_right (
>      fun (n, fields) acc ->
> -- 
The patches are fine, and so ACK.

There is a possible problem in that the string isn't translated.  (I'm
not totally clear if yyerror passes in a translated 'msg', and in any
case the fprintf doesn't call gettext, so maybe this is not relevant).

If translating the message is desirable, then possibly something like:

  Ini_reader.read_init ~prog ~ignore_errors:true file

with the ~ignore_errors flag causing the string to be printed like
this:

  if (! Bool_val (ignore_errorsv)) {
    fprintf (stderr, _("%s%s%ssyntax error at line %d: %s\n"), ..);
  } else {
    fprintf (stderr, _("%s%s%ssyntax error at line %d: %s
(ignored)\n"), ..);
  }

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
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