Re: [Libvir] [PATCH] Device attach/detach on virsh(XML version)
Hi Rich,
On Tue, 2007-05-22 at 15:10 +0100, Richard W.M. Jones wrote:
+static char *
+readFile (vshControl *ctl, const char *filename)
+{
+ char *buffer = NULL, *oldbuffer;
+ int len = 0, fd, r;
+ char b[1024];
+
+ fd = open (filename, O_RDONLY);
+ if (fd == -1) {
+ file_error:
+ vshError (ctl, FALSE, "%s: %s", filename, strerror (errno));
+ error:
+ if (buffer) free (buffer);
+ if (fd >= 0) close (fd);
+ return NULL;
+ }
+
+ for (;;) {
+ r = read (fd, b, sizeof b);
+ if (r == -1) goto file_error;
+ if (r == 0) break; /* End of file. */
+ oldbuffer = buffer;
+ buffer = realloc (buffer, len+r);
+ if (buffer == NULL) {
+ out_of_memory:
+ vshError (ctl, FALSE, "realloc: %s", strerror (errno));
+ goto error;
+ }
+ memcpy (buffer+len, b, r);
+ len += r;
+ }
+
+ buffer = realloc (buffer, len+1);
+ if (buffer == NULL) goto out_of_memory;
+ buffer[len] = '\0';
+ return buffer;
+}
Truly, the way you're using gotos here gives me a serious headache.
Following the logic of the function involves jumping back and forth
around the code way too much. I noticed you using that style in the
remote patch too.
I'd suggest something more like:
static char *
readFile(vshControl *ctl, const char *filename)
{
char *retval;
int len = 0, fd;
if ((fd = open(filename, O_RDONLY)) < 0) {
vshError (ctl, FALSE, "Failed to open '%s': %s",
filename, strerror (errno));
return NULL;
}
if (!(retval = malloc(len + 1)))
goto out_of_memory;
while (1) {
char buffer[1024];
char *new;
int ret;
if ((ret = read(fd, buffer, sizeof(buffer))) == 0)
break;
if (ret == -1) {
if (errno == EINTR)
continue;
vshError (ctl, FALSE, "Failed to open '%s': %s",
filename, strerror (errno));
goto error;
}
if (!(new = realloc(retval, len + ret + 1)))
goto out_of_memory;
retval = new;
memcpy(retval + len, buffer, ret);
len += ret;
}
retval[len] = '\0';
return buffer;
out_of_memory:
vshError (ctl, FALSE, "Error allocating memory: %s",
strerror(errno));
error:
if (retval)
free(retval);
close(fd);
return NULL;
}
You can read this version straight through without having to jump back
to an earlier part of the function.
Cheers,
Mark.