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; > +} -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list