[Qemu-devel] [PATCH 01/16] cutils: tighten qemu_parse_fd()

2014-04-10 Thread Laszlo Ersek
qemu_parse_fd() used to handle at least the following strings incorrectly:
o -2: simply let through
o 2147483648: returned as LONG_MAX==INT_MAX on ILP32 (with ERANGE
ignored); implementation-defined behavior on LP64

Signed-off-by: Laszlo Ersek ler...@redhat.com
---
 util/cutils.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/util/cutils.c b/util/cutils.c
index b337293..dbe7412 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -22,10 +22,12 @@
  * THE SOFTWARE.
  */
 #include qemu-common.h
 #include qemu/host-utils.h
 #include math.h
+#include limits.h
+#include errno.h
 
 #include qemu/sockets.h
 #include qemu/iov.h
 #include net/net.h
 
@@ -455,15 +457,20 @@ int parse_uint_full(const char *s, unsigned long long 
*value, int base)
 return 0;
 }
 
 int qemu_parse_fd(const char *param)
 {
-int fd;
-char *endptr = NULL;
+long fd;
+char *endptr;
 
+errno = 0;
 fd = strtol(param, endptr, 10);
-if (*endptr || (fd == 0  param == endptr)) {
+if (param == endptr /* no conversion performed */||
+errno != 0  /* not representable as long; possibly others */ ||
+*endptr != '\0' /* final string not empty */ ||
+fd  0  /* invalid as file descriptor */ ||
+fd  INT_MAX/* not representable as int */) {
 return -1;
 }
 return fd;
 }
 
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH 01/16] cutils: tighten qemu_parse_fd()

2014-04-10 Thread Eric Blake
On 04/10/2014 02:24 AM, Laszlo Ersek wrote:
 qemu_parse_fd() used to handle at least the following strings incorrectly:
 o -2: simply let through
 o 2147483648: returned as LONG_MAX==INT_MAX on ILP32 (with ERANGE
 ignored); implementation-defined behavior on LP64
 
 Signed-off-by: Laszlo Ersek ler...@redhat.com
 ---
  util/cutils.c | 13 ++---
  1 file changed, 10 insertions(+), 3 deletions(-)

I still think qemu should follow libvirt's lead of wrapping ALL uses of
strto*l behind sane wrappers, since this is not the only place in the
code base affected by misuse of the function - but that's a story for
another day.

  
 +errno = 0;
  fd = strtol(param, endptr, 10);
 -if (*endptr || (fd == 0  param == endptr)) {
 +if (param == endptr /* no conversion performed */||
 +errno != 0  /* not representable as long; possibly others */ ||
 +*endptr != '\0' /* final string not empty */ ||
 +fd  0  /* invalid as file descriptor */ ||
 +fd  INT_MAX/* not representable as int */) {
  return -1;

Your comments make it particularly obvious that YOU know how to properly
use this function, and hopefully teach future readers. :)

Reviewed-by: Eric Blake ebl...@redhat.com

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature