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

Re: [Libguestfs] [PATCH] Add command trace functionality



Richard W.M. Jones wrote:
...
A few small suggestions on the generated code:

> diff --git a/src/generator.ml b/src/generator.ml
...
> +    pr "    printf (\"%%s\", \"%s\");\n" shortname;
> +    List.iter (
> +      function
> +      | String n			(* strings *)
> +      | Device n
> +      | Pathname n
> +      | Dev_or_Path n
> +      | FileIn n
> +      | FileOut n ->
> +	  (* guestfish doesn't support string escaping, so neither do we *)
> +	  pr "    printf (\" \\\"%%s\\\"\", %s);\n" n
> +      | OptString n ->			(* string option *)
> +	  pr "    if (%s) printf (\" \\\"%%s\\\"\", %s);\n" n n;
> +	  pr "    else printf (\" null\");\n"
> +      | StringList n
> +      | DeviceList n ->			(* string list *)
> +	  pr "    printf (\"\\\"\");\n";

How about this, instead?

    	  pr "    putchar ('\"');\n";

> +	  pr "    for (i = 0; %s[i]; ++i) {\n" n;
> +	  pr "      if (i > 0) putchar (' ');\n";
> +	  pr "      printf (\"%%s\", %s[i]);\n" n;

The minimalist in me prefers to use fputs in cases like this:
    	  pr "      fputs (%s[i], stdout);\n" n;

> +	  pr "    }\n";
> +	  pr "    printf (\"\\\"\");\n";

Same as above.

> +      | Bool n ->			(* boolean *)
> +	  pr "    if (%s) printf (\" true\");\n" n;
> +	  pr "    else printf (\" false\");\n"

you might like to replace the two lines above with this one:
    	  pr "    printf (%s ? \" true\" : \" false\");\n" n;
or this,
    	  pr "    fputs (%s ? \" true\" : \" false\", stdout);\n" n;

> +      | Int n ->			(* int *)
> +	  pr "    printf (\" %%d\", %s);\n" n
> +    ) (snd style);
> +    pr "    printf (\"\\n\");\n";
> +    pr "  }\n";
> +    pr "\n";
> +  in


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