]> xenbits.xensource.com Git - xen.git/commitdiff
blktap2: Fix naked unchecked uses of read/write/chdir.
authorKeir Fraser <keir@xen.org>
Mon, 14 May 2012 15:59:12 +0000 (16:59 +0100)
committerKeir Fraser <keir@xen.org>
Mon, 14 May 2012 15:59:12 +0000 (16:59 +0100)
These cause warnings under warn_unused_result, and for read/write we
ought to deal with partial io results.

Signed-off-by: Keir Fraser <keir@xen.org>
xen-unstable changeset:   25299:01d64a3dea71
xen-unstable date:        Fri May 11 18:30:29 2012 +0100

blktap2: Fix another uninitialised value error

gcc  -O1 -fno-omit-frame-pointer -m32 -march=i686 -g
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
-Wdeclaration-after-statement   -D__XEN_TOOLS__ -MMD -MF
.block-remus.o.d -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls
-mno-tls-direct-seg-refs -Werror -g -Wno-unused -fno-strict-aliasing
-I../include -I../drivers
-I/home/osstest/build.12828.build-i386/xen-unstable/tools/blktap2/drivers/../../../tools/libxc
-I/home/osstest/build.12828.build-i386/xen-unstable/tools/blktap2/drivers/../../../tools/include
-D_GNU_SOURCE -DUSE_NFS_LOCKS  -c -o block-remus.o block-remus.c

block-remus.c: In function 'ramdisk_flush':
block-remus.c:508: error: 'buf' may be used uninitialized in this
function
make[5]: *** [block-remus.o] Error 1

This is because gcc can see that merge_requests doesn't always set
*mergedbuf but gcc isn't able to prove that it always does so if
merge_requests returns 0 and that in that case the value of
ramdisk_flush::buf isn't used.

This is too useful a warning to disable, despite the occasional false
positive of this form.  The conventional approach is to suppress the
warning by explicitly initialising the variable to 0.

This has just come to light because 25275:27d63b9f111a reenabled
optimisation for this area of code, and gcc's data flow analysis
(which is required to trigger the uninitialised variable warning) only
occurs when optimisation is turned on.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset:   25281:60064411a8a9
xen-unstable date:        Thu May 10 14:26:14 2012 +0100

blktap2: Do not build with -O0

Signed-off-by: Keir Fraser <keir@xen.org>
xen-unstable changeset:   25275:27d63b9f111a
xen-unstable date:        Thu May 10 11:22:18 2012 +0100

blktap2: Fix uninitialised value error.

Signed-off-by: Keir Fraser <keir@xen.org>
xen-unstable changeset:   25274:cb82b5aa73bd
xen-unstable date:        Thu May 10 11:21:59 2012 +0100

tools/blktap2: fix out of bounds access in block-log.c

block-log.c: In function 'ctl_close_sock':
block-log.c:363:23: warning: array subscript is above array bounds
[-Warray-bounds]

Adjust loop condition in ctl_close_sock() to fix warning.
Adjust array acccess in ctl_close() to actually access the array
member.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Keir Fraser <keir@xen.org>
xen-unstable changeset:   25273:83a02f225bde
xen-unstable date:        Thu May 10 11:20:04 2012 +0100

tools/blktap2: fix build errors caused by Werror in
vhd_journal_write_entry

-O2 -Wall -Werror triggers these warnings:

libvhd-journal.c: In function 'vhd_journal_write_entry':
libvhd-journal.c:335: warning: statement with no effect

Really return the error from vhd_journal_write() to caller.

v2:
 - simplify the patch by just adding the missing return statement

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Committed-by: Keir Fraser <keir@xen.org>
xen-unstable changeset:   25272:ca02580986d2
xen-unstable date:        Thu May 10 11:19:05 2012 +0100

12 files changed:
tools/blktap2/drivers/Makefile
tools/blktap2/drivers/block-log.c
tools/blktap2/drivers/block-qcow.c
tools/blktap2/drivers/block-remus.c
tools/blktap2/drivers/tapdisk-diff.c
tools/blktap2/drivers/tapdisk-log.c
tools/blktap2/drivers/tapdisk-queue.c
tools/blktap2/drivers/tapdisk-stream.c
tools/blktap2/drivers/tapdisk-utils.c
tools/blktap2/drivers/tapdisk-utils.h
tools/blktap2/drivers/tapdisk2.c
tools/blktap2/vhd/lib/libvhd-journal.c

index b718c0839fef609a685bc3447403e75b4660d4fa..cc36fc42fbefb9e173381e70223554970752fd2c 100644 (file)
@@ -9,7 +9,7 @@ QCOW_UTIL  = img2qcow qcow-create qcow2raw
 LOCK_UTIL  = lock-util
 INST_DIR   = $(SBINDIR)
 
-CFLAGS    += -Werror -g -O0
+CFLAGS    += -Werror -g
 CFLAGS    += -Wno-unused
 CFLAGS    += -fno-strict-aliasing
 CFLAGS    += -I$(BLKTAP_ROOT)/include -I$(BLKTAP_ROOT)/drivers
index c9612a4b9b41afb00c10fe331ceb2471953233a3..a03565b65e0b53c64385cbf4c3b4bdcc2ff369a2 100644 (file)
@@ -347,11 +347,11 @@ static int ctl_open(struct tdlog_state* s, const char* name)
 static int ctl_close(struct tdlog_state* s)
 {
   while (s->connected) {
+    s->connected--;
     tapdisk_server_unregister_event(s->connections[s->connected].id);
     close(s->connections[s->connected].fd);
     s->connections[s->connected].fd = -1;
     s->connections[s->connected].id = 0;
-    s->connected--;
   }
 
   if (s->ctl.fd >= 0) {
@@ -382,7 +382,7 @@ static int ctl_close_sock(struct tdlog_state* s, int fd)
 {
   int i;
 
-  for (i = 0; i <= s->connected; i++) {
+  for (i = 0; i < s->connected; i++) {
     if (s->connections[i].fd == fd) {
       tapdisk_server_unregister_event(s->connections[i].id);
       close(s->connections[i].fd);
index ac55ac938742f0bb32203477ecd407164f6ef2aa..d5053d41748bec9b7e43488e64b550f192fe77f7 100644 (file)
@@ -1428,7 +1428,7 @@ int tdqcow_get_parent_id(td_driver_t *driver, td_disk_id_t *id)
 {
        off_t off;
        char *buf, *filename;
-       int len, secs, type, err = -EINVAL;
+       int len, secs, type = 0, err = -EINVAL;
        struct tdqcow_state *child  = (struct tdqcow_state *)driver->data;
 
        if (!child->backing_file_offset)
index 0af01f13e79cde1949f8bb2e24778a79727ce707..079588d820064c0cd244551192e97eddfd3a2181 100644 (file)
@@ -505,7 +505,7 @@ fail:
 static int ramdisk_flush(td_driver_t *driver, struct tdremus_state* s)
 {
        uint64_t* sectors;
-       char* buf;
+       char* buf = NULL;
        uint64_t base, batchlen;
        int i, j, count = 0;
 
index 1d2ba8e682bc65ce3896421688a26c373f18956e..056d4c9f60815f9e058d4d7c0641e0188663f633 100644 (file)
@@ -39,6 +39,7 @@
 #include "tapdisk-vbd.h"
 #include "tapdisk-server.h"
 #include "tapdisk-disktype.h"
+#include "tapdisk-utils.h"
 #include "libvhd.h"
 
 #define POLL_READ                        0
@@ -170,7 +171,7 @@ tapdisk_stream_poll_clear(struct tapdisk_stream_poll *p)
 {
        int dummy;
 
-       read(p->pipe[POLL_READ], &dummy, sizeof(dummy));
+       read_exact(p->pipe[POLL_READ], &dummy, sizeof(dummy));
        p->set = 0;
 }
 
@@ -180,7 +181,7 @@ tapdisk_stream_poll_set(struct tapdisk_stream_poll *p)
        int dummy = 0;
 
        if (!p->set) {
-               write(p->pipe[POLL_WRITE], &dummy, sizeof(dummy));
+               write_exact(p->pipe[POLL_WRITE], &dummy, sizeof(dummy));
                p->set = 1;
        }
 }
index 1220bf927e917e3a7a71b534ae5caa3f4005f4a0..d14da3241570621ccccf7d5a265801fd0b063760 100644 (file)
@@ -37,6 +37,7 @@
 #include <sys/time.h>
 
 #include "tapdisk-log.h"
+#include "tapdisk-utils.h"
 
 #define MAX_ENTRY_LEN      512
 #define MAX_ERROR_MESSAGES 16
@@ -247,7 +248,7 @@ tlog_flush(void)
        wsize = ((size + 511) & (~511));
 
        memset(tapdisk_log.buf + size, '\n', wsize - size);
-       write(fd, tapdisk_log.buf, wsize);
+       write_exact(fd, tapdisk_log.buf, wsize);
 
        tapdisk_log.p = tapdisk_log.buf;
 
index b6394bfdad0253d18d1bb27966cc423c05c0237a..1a9403828b4f61e1348c29ea3433caf550a81ac8 100644 (file)
@@ -435,7 +435,7 @@ tapdisk_lio_ack_event(struct tqueue *queue)
        uint64_t val;
 
        if (lio->flags & LIO_FLAG_EVENTFD)
-               read(lio->event_fd, &val, sizeof(val));
+               read_exact(lio->event_fd, &val, sizeof(val));
 }
 
 static void
index 1795abddfd9b98c6236d030221ef5f514e2f16ec..b5b0fa786911f0b4f23a7007a717d21cb81ee0c1 100644 (file)
@@ -38,6 +38,7 @@
 #include "tapdisk-vbd.h"
 #include "tapdisk-server.h"
 #include "tapdisk-disktype.h"
+#include "tapdisk-utils.h"
 
 #define POLL_READ                        0
 #define POLL_WRITE                       1
@@ -145,7 +146,7 @@ tapdisk_stream_poll_clear(struct tapdisk_stream_poll *p)
 {
        int dummy;
 
-       read(p->pipe[POLL_READ], &dummy, sizeof(dummy));
+       read_exact(p->pipe[POLL_READ], &dummy, sizeof(dummy));
        p->set = 0;
 }
 
@@ -155,7 +156,7 @@ tapdisk_stream_poll_set(struct tapdisk_stream_poll *p)
        int dummy = 0;
 
        if (!p->set) {
-               write(p->pipe[POLL_WRITE], &dummy, sizeof(dummy));
+               write_exact(p->pipe[POLL_WRITE], &dummy, sizeof(dummy));
                p->set = 1;
        }
 }
@@ -203,7 +204,7 @@ tapdisk_stream_print_request(struct tapdisk_stream *s,
 {
        unsigned long idx = (unsigned long)tapdisk_stream_request_idx(s, sreq);
        char *buf = (char *)MMAP_VADDR(s->vbd->ring.vstart, idx, 0);
-       write(s->out_fd, buf, sreq->secs << SECTOR_SHIFT);
+       write_exact(s->out_fd, buf, sreq->secs << SECTOR_SHIFT);
 }
 
 static void
index 1b15f12154267c7179a6a12888f0e965a997fdd0..44840efbf844bfdb8b7b15a8686acabd064f8369 100644 (file)
@@ -175,3 +175,40 @@ int tapdisk_linux_version(void)
 }
 
 #endif
+int read_exact(int fd, void *data, size_t size)
+{
+    size_t offset = 0;
+    ssize_t len;
+
+    while ( offset < size )
+    {
+        len = read(fd, (char *)data + offset, size - offset);
+        if ( (len == -1) && (errno == EINTR) )
+            continue;
+        if ( len == 0 )
+            errno = 0;
+        if ( len <= 0 )
+            return -1;
+        offset += len;
+    }
+
+    return 0;
+}
+
+int write_exact(int fd, const void *data, size_t size)
+{
+    size_t offset = 0;
+    ssize_t len;
+
+    while ( offset < size )
+    {
+        len = write(fd, (const char *)data + offset, size - offset);
+        if ( (len == -1) && (errno == EINTR) )
+            continue;
+        if ( len <= 0 )
+            return -1;
+        offset += len;
+    }
+
+    return 0;
+}
index 7b7c3d5e92524d75d66415c34bac9d725afcde79..aced8ef492377fd09ce86f01fddd5b7381591064 100644 (file)
@@ -39,4 +39,7 @@ int tapdisk_namedup(char **, const char *);
 int tapdisk_get_image_size(int, uint64_t *, uint32_t *);
 int tapdisk_linux_version(void);
 
+int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */
+int write_exact(int fd, const void *data, size_t size);
+
 #endif
index d30f73984ac60e2d4478b345342445a69c633ae6..531308613c1ed2f7291f2a72be21f6a358b8dcef 100644 (file)
@@ -79,7 +79,12 @@ main(int argc, char *argv[])
        if (optind != argc)
                usage(argv[0], EINVAL);
 
-       chdir("/");
+       if (chdir("/")) {
+               DPRINTF("failed to chdir(/): %d\n", errno);
+               err = 1;
+               goto out;
+       }
+
        tapdisk_start_logging("tapdisk2");
 
        err = tapdisk_server_init();
index 24383ff62b299cbca0fb3016e73f37bf82b02f72..2cd0c9a6afc35ab115ce4bb45ac92564fe9e533a 100644 (file)
@@ -332,7 +332,7 @@ vhd_journal_write_entry(vhd_journal_t *j, vhd_journal_entry_t *entry)
 
        err = vhd_journal_write(j, &e, sizeof(vhd_journal_entry_t));
        if (err)
-               err;
+               return err;
 
        return 0;
 }