There were several unchecked syscalls in this function, along with the
at-least-theoretical risk of a file descriptor leak, so I rewrote this
function to avoid all that, using a stream rather than a bare file
descriptor.
Subject: [PATCH] Rewrite openvzSetUUID.
* src/openvz_conf.c (openvzSetUUID): Rewrite to avoid unchecked
lseek, write, and close as well as a potential file descriptor leak.
Regarding style, the if (expr1 + expr2 + expr3) below might look
odd, but it's better than "if (expr1 || expr2 || expr3)", which
loses if expr1 or expr2 is true, since it short-circuits and
skips expr3, which is the required fclose call.
I could have used "|", but that seemed worse,
and I didn't like the duplication in
if (expr1)
ret = -1;
if (expr2)
ret = -1;
if (expr3)
ret = -1;
In an early iteration, I even wrote this (which still
has too much duplication but isn't that bad):
ret = 0;
ret |= e1;
ret |= e2;
ret |= e3;
Anyway, just trying to avoid a drawn-out discussion on style....
Signed-off-by: Jim Meyering <meyering@xxxxxxxxxx>
---
src/openvz_conf.c | 27 ++++++++++++++-------------
1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/src/openvz_conf.c b/src/openvz_conf.c
index a274223..1f45c24 100644
--- a/src/openvz_conf.c
+++ b/src/openvz_conf.c
@@ -680,7 +680,7 @@ openvzSetUUID(int vpsid)
char uuidstr[VIR_UUID_STRING_BUFLEN];
unsigned char uuid[VIR_UUID_BUFLEN];
char *conf_dir;
- int fd, ret;
+ int ret;
conf_dir = openvzLocateConfDir();
if (conf_dir == NULL)
@@ -688,26 +688,27 @@ openvzSetUUID(int vpsid)
sprintf(conf_file, "%s/%d.conf", conf_dir, vpsid);
free(conf_dir);
- fd = open(conf_file, O_RDWR);
- if(fd == -1)
- return -1;
-
ret = openvzGetVPSUUID(vpsid, uuidstr);
- if(ret == -1)
+ if (ret)
return -1;
- if(uuidstr[0] == 0) {
+ if (uuidstr[0] == 0) {
+ FILE *fp = fopen(conf_file, "a");
+ if (fp == NULL)
+ return -1;
+
virUUIDGenerate(uuid);
virUUIDFormat(uuid, uuidstr);
- lseek(fd, 0, SEEK_END);
- write(fd, "\n#UUID: ", 8);
- write(fd, uuidstr, strlen(uuidstr));
- write(fd, "\n", 1);
- close(fd);
+ /* Record failure if any of these fails,
+ and be careful always to close the stream. */
+ if ((fseek(fp, 0, SEEK_END) < 0)
+ + (fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0);
+ + (fclose(fp) == EOF))
+ ret = -1;
}
- return 0;
+ return ret;
}
/*
--
1.5.4.2.134.g82883
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list