Satoru SATOH wrote:
Here is updated version of the patch adds the files implements dnsmasq (hostsfile) module [1] based on advices from Jim-san.
[1] https://www.redhat.com/archives/libvir-list/2010-April/msg01003.html [2] https://www.redhat.com/archives/libvir-list/2010-April/msg01046.html
Thank you for the update. Here are a few more suggestions:
+static int +hostsfileWrite(const char *path, + dnsmasqDhcpHost *hosts, + unsigned int nhosts) +{ + char *tmp = NULL; + FILE *f; + bool istmp = true; + unsigned int i; + int rc;
Please initialize "rc" here, int rc = 0; so that, below(at the end)...
+ if (nhosts == 0) + return 0; + + if (virAsprintf(&tmp, "%s.new", path) < 0) + return -1;
Please change that to "return ENOMEM;". At first I thought it should be "return errno;", but neither virAsprintf nor vasprintf/asprintf specify if/how errno is set upon failure.
+ if (!(f = fopen(tmp, "w"))) { + istmp = false; + if (!(f = fopen(path, "w"))) { + rc = errno; + goto cleanup; + } + } + + for (i = 0; i < nhosts; i++) { + if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) { + rc = errno; + fclose(f); + + if (istmp) + unlink(tmp); + + goto cleanup; + } + } + + if (fclose(f) == EOF) { + rc = errno; + goto cleanup; + } + + if (istmp) { + if (rename(tmp, path) < 0) { + rc = errno; + unlink(tmp); + goto cleanup; + } + + if (unlink(tmp) < 0) { + rc = errno; + goto cleanup; + } + } + + VIR_FREE(tmp); + + return 0;
... you can remove the "VIR_FREE..." and "return 0" above.
+ cleanup: + VIR_FREE(tmp); + + return rc; +} ... +int +dnsmasqReload(pid_t pid) +{ + if (kill(pid, SIGHUP) != 0) { + virReportSystemError(errno, + _("Failed to make dnsmasq (PID: %d) reloading config files.\n"),
s/reloading/reload/
+ pid); + return -1; + } + + return 0; +}