Re: [libvirt] [PATCH 1/4] Make avahi startup more robust.

On 04/21/2010 12:25 PM, Eric Blake wrote: > On 04/21/2010 10:03 AM, Chris Lalancette wrote: >> If the hostname of the current virtualization machine >> could not be resolved, then libvirtd would fail to >> start. However, for disconnected operation (on a laptop, >> for instance) the hostname may very legitimately not >> be resolvable. This patch makes it so that if we can't >> resolve the hostname, avahi doesn't fail, it just uses >> a less useful MDNS string. > > ACK on the concept, but fix the corner-case memory leak before pushing. > >> >> if (!mdns_name) { >> - char groupname[64], *localhost, *tmp; >> + char *groupname, *localhost, *tmp; >> /* Extract the host part of the potentially FQDN */ >> localhost = virGetHostname(NULL); > > Here, localhost can be allocated... > >> if (localhost == NULL) >> + ret = virAsprintf(&groupname, "Virtualization Host"); >> + else { >> + if ((tmp = strchr(localhost, '.'))) >> + *tmp = '\0'; >> + ret = virAsprintf(&groupname, "Virtualization Host %s", >> + localhost); > > then groupname fails... > >> + } >> + if (ret < 0) { >> + virReportOOMError(); >> goto cleanup; > > ...and we leak localhost. > >> - >> - if ((tmp = strchr(localhost, '.'))) >> - *tmp = '\0'; >> - snprintf(groupname, sizeof(groupname)-1, "Virtualization Host %s", localhost); >> - groupname[sizeof(groupname)-1] = '\0'; >> + } >> group = libvirtd_mdns_add_group(server->mdns, groupname); >> VIR_FREE(localhost); >> + VIR_FREE(groupname); > > But on success, there is no leak. > Oops, good call. I'll respin the patch with that change.