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;