[Libguestfs] [PATCH] generator: Fix API of functions that return RBufferOut

[Jim Meyering]

Richard W.M. Jones wrote:
> Subject: [PATCH] generator: Fix API of functions that return RBufferOut
>
> (NB: The API / ABI doesn't actually change here - it's just made much
> simpler to use).
>
> The API for RBufferOut functions was unexpectedly hard to use in the
> case where a zero-length buffer might be returned.  For discussion on
> this see:
...
> diff --git a/src/generator.ml b/src/generator.ml
> index c5f21df..2317541 100644
> --- a/src/generator.ml
> +++ b/src/generator.ml
> @@ -5107,7 +5109,7 @@ and generate_client_actions () =
>
>  #define error guestfs_error
>  //#define perrorf guestfs_perrorf
> -//#define safe_malloc guestfs_safe_malloc
> +#define safe_malloc guestfs_safe_malloc
>  #define safe_realloc guestfs_safe_realloc
>  //#define safe_strdup guestfs_safe_strdup
>  #define safe_memdup guestfs_safe_memdup
> @@ -5396,8 +5398,20 @@ check_state (guestfs_h *g, const char *caller)
>             pr "  /* caller will free this */\n";
>             pr "  return safe_memdup (g, &ret.%s, sizeof (ret.%s));\n" n n
>         | RBufferOut n ->
> -           pr "  *size_r = ret.%s.%s_len;\n" n n;
> -           pr "  return ret.%s.%s_val; /* caller will free */\n" n n
> +	   pr "  /* RBufferOut is tricky: If the buffer is zero-length, then\n";
> +	   pr "   * _val might be NULL here.  To make the API saner for\n";
> +	   pr "   * callers, we turn this case into a unique pointer (using\n";
> +	   pr "   * malloc(1)).\n";
> +	   pr "   */\n";
> +	   pr "  if (ret.%s.%s_len > 0) {\n" n n;
> +           pr "    *size_r = ret.%s.%s_len;\n" n n;
> +           pr "    return ret.%s.%s_val; /* caller will free */\n" n n;
> +	   pr "  } else {\n";
> +	   pr "    free (ret.%s.%s_val);\n" n n;
> +	   pr "    char *p = safe_malloc (g, 1);\n";
> +           pr "    *size_r = ret.%s.%s_len;\n" n n;
> +           pr "    return p;\n";
> +	   pr "  }\n";

Hi Rich,

That looks fine.

You can hoist this assignment to preced the 'if' test,
since the same statement is in both the then and else blocks:

  *size_r = ret.%s.%s_len;\n" n n;

Also, note the mix of indentatation spaces and TABs in added lines.