Hi Rich, I've changed part of generator.ml to look like this: (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"; 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 -> pr " char *%s = args.%s;\n" n n; pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);" n; | String n -> pr " char *%s = args.%s;\n" n 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" ); Note the 4-x repeated statement: pr " char *%s = args.%s;\n" n n; What's the best way to avoid that?
On Wed, Aug 12, 2009 at 10:07:16AM +0200, Jim Meyering wrote:
Hi Rich,
I've changed part of generator.ml to look like this:
(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"; 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 -> pr " char *%s = args.%s;\n" n n; pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);" n; | String n -> pr " char *%s = args.%s;\n" n 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" );
Note the 4-x repeated statement:
pr " char *%s = args.%s;\n" n n;
What's the best way to avoid that?
Just created a nested function: let pr_args n = pr " char *%s = args.%s;\n" n n in (* ... *) pr_args n You can create functions anywhere, at any level of nesting, even inside expressions, and nested functions are common and a Good Thing. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
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. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
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" );
On Wed, Aug 12, 2009 at 12:09:25PM +0200, Jim Meyering wrote:
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" );
Looks fine. You're only saving a few characters by factoring out that function though. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
participants (2)
-
Jim Meyering -
Richard W.M. Jones