[libvirt] [PATCH 1/3] Add dnsmasq module (v2.1)
Jim Meyering
jim at meyering.net
Fri Apr 23 20:07:55 UTC 2010
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;
> +}
More information about the libvir-list
mailing list