Re: [libvirt] [PATCH 1/3] Add dnsmasq module (v2.1)
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;
+}