(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:
https://www.redhat.com/archives/libguestfs/2009-November/thread.html#00115
This commit ensures that in the zero-length buffer case, the return
value is never NULL. Thus code is now able to just check if the return
value == NULL to indicate an error, which is simpler for all concerned.
The implementation of this is, however, more complex because we have
to be careful about this case inside both the daemon and the library
code, which is what this commit does.
This has passed a full round of tests.
if (size > limit)
size = limit;
if (size > limit)
size = limit;
- /* Note the correct error handling here is tricky, because in the
- * case where the call returns a zero-length buffer, it might return
- * NULL. However it won't adjust rsize along the error path, so we
- * can set rsize to something beforehand and use that as a flag.
- */
- rsize = 1;
r = guestfs_pread (g, path, size, offset, &rsize);
r = guestfs_pread (g, path, size, offset, &rsize);
- if (rsize == 1 && r == NULL)
return error ();
/* This should never happen, but at least it stops us overflowing
return error ();
/* This should never happen, but at least it stops us overflowing
("pread", (RBufferOut "content", [Pathname "path"; Int "count"; Int64 "offset"]), 207, [ProtocolLimitWarning],
[InitISOFS, Always, TestOutputBuffer (
("pread", (RBufferOut "content", [Pathname "path"; Int "count"; Int64 "offset"]), 207, [ProtocolLimitWarning],
[InitISOFS, Always, TestOutputBuffer (
- [["pread"; "/known-4"; "1"; "3"]], "\n")],
+ [["pread"; "/known-4"; "1"; "3"]], "\n");
+ InitISOFS, Always, TestOutputBuffer (
+ [["pread"; "/empty"; "0"; "100"]], "")],
"read part of a file",
"\
This command lets you read part of a file. It reads C<count>
"read part of a file",
"\
This command lets you read part of a file. It reads C<count>
#define error guestfs_error
//#define perrorf guestfs_perrorf
#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
#define safe_realloc guestfs_safe_realloc
//#define safe_strdup guestfs_safe_strdup
#define safe_memdup guestfs_safe_memdup
pr " /* caller will free this */\n";
pr " return safe_memdup (g, &ret.%s, sizeof (ret.%s));\n" n n
| RBufferOut n ->
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";
| RStruct (_, typ) -> pr " guestfs_int_%s *r;\n" typ; "NULL"
| RStructList (_, typ) -> pr " guestfs_int_%s_list *r;\n" typ; "NULL"
| RBufferOut _ ->
| RStruct (_, typ) -> pr " guestfs_int_%s *r;\n" typ; "NULL"
| RStructList (_, typ) -> pr " guestfs_int_%s_list *r;\n" typ; "NULL"
| RBufferOut _ ->
+ pr " size_t size = 1;\n";
pr " char *r;\n";
"NULL" in
pr " char *r;\n";
"NULL" in
generate_c_call_args (fst style, args');
pr ";\n";
generate_c_call_args (fst style, args');
pr ";\n";
- pr " if (r == %s)\n" error_code;
- pr " /* do_%s has already called reply_with_error */\n" name;
- pr " goto done;\n";
- pr "\n";
+ (match fst style with
+ | RErr | RInt _ | RInt64 _ | RBool _
+ | RConstString _ | RConstOptString _
+ | RString _ | RStringList _ | RHashtable _
+ | RStruct (_, _) | RStructList (_, _) ->
+ pr " if (r == %s)\n" error_code;
+ pr " /* do_%s has already called reply_with_error */\n" name;
+ pr " goto done;\n";
+ pr "\n"
+ | RBufferOut _ ->
+ pr " /* size == 0 && r == NULL could be a non-error case (just\n";
+ pr " * an ordinary zero-length buffer), so be careful ...\n";
+ pr " */\n";
+ pr " if (size == 1 && r == %s)\n" error_code;
+ pr " /* do_%s has already called reply_with_error */\n" name;
+ pr " goto done;\n";
+ pr "\n"
+ );
(* If there are any FileOut parameters, then the impl must
* send its own reply.
(* If there are any FileOut parameters, then the impl must
* send its own reply.