[Libguestfs] Re: factorization would be nice

[Jim Meyering]

Richard W.M. Jones wrote:
> On Wed, Aug 12, 2009 at 09:15:31AM +0100, Richard W.M. Jones wrote:
>> On Wed, Aug 12, 2009 at 10:07:16AM +0200, Jim Meyering wrote:
>> >            List.iter (
>> >              function
>> >              | Pathname n ->
>> >                  pr "  char *%s = args.%s;\n" n n;
>> >                  pr "  ABS_PATH (%s, goto done);\n" n;
>> >              | Device n ->
>> >                  pr "  char *%s = args.%s;\n" n n;
>> >                  pr "  RESOLVE_DEVICE (%s, goto done);" n;
>> > 	     | Dev_or_Path n ->
>
> On thing to add here:
>
> The 'n's in this code are all different bindings.  So there's no way
> to get a closure which captures the values of all 'n's.
>
> Thus in other code (but not in this case) you could have written:
>
>   let pr_args () =
>     pr "  char *%s = args.%s\n" n n
>   in
>
> where the value of 'n' would be captured in the closure 'pr_args', and
> not have to be given explicitly each time.

Thanks for the explanation.
Here's what I have now:

      (match snd style with
       | [] -> ()
       | args ->
           pr "  memset (&args, 0, sizeof args);\n";
           pr "\n";
           pr "  if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name;
           pr "    reply_with_error (\"%%s: daemon failed to decode procedure arguments\", \"%s\");\n" name;
           pr "    return;\n";
           pr "  }\n";
	   let pr_args n =
	     pr "  char *%s = args.%s;\n" n n
	   in
           List.iter (
             function
             | Pathname n ->
                 pr_args n;
                 pr "  ABS_PATH (%s, goto done);\n" n;
             | Device n ->
                 pr_args n;
                 pr "  RESOLVE_DEVICE (%s, goto done);" n;
	     | Dev_or_Path n ->
                 pr_args n;
                 pr "  REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);" n;
             | String n -> pr_args n
             | OptString n -> pr "  %s = args.%s ? *args.%s : NULL;\n" n n n
             | StringList n ->
                 pr "  %s = realloc (args.%s.%s_val,\n" n n n;
                 pr "                sizeof (char *) * (args.%s.%s_len+1));\n" n n;
                 pr "  if (%s == NULL) {\n" n;
                 pr "    reply_with_perror (\"realloc\");\n";
                 pr "    goto done;\n";
                 pr "  }\n";
                 pr "  %s[args.%s.%s_len] = NULL;\n" n n n;
                 pr "  args.%s.%s_val = %s;\n" n n n;
             | Bool n -> pr "  %s = args.%s;\n" n n
             | Int n -> pr "  %s = args.%s;\n" n n
             | FileIn _ | FileOut _ -> ()
           ) args;
           pr "\n"
      );