On Tue, 7 Jul 2015 at 16:49, Alexander Graf <ag...@suse.de> wrote:
>
> From: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
>
> Each hardware instance has a platform unique location code. The OF
> device tree that describes a part of a hardware entity must include
> the “ibm,loc-code” property with a value that represents the location
> code for that hardware entity.
>
> Populate ibm,loc-code.
Ancient patch, but Coverity has just noticed a bug in it
which is still present in current QEMU (CID 1460454):
> +static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice
> *pdev)
> +{
> + char *path = NULL, *buf = NULL, *host = NULL;
> +
> + /* Get the PCI VFIO host id */
> + host = object_property_get_str(OBJECT(pdev), "host", NULL);
> + if (!host) {
> + goto err_out;
> + }
> +
> + /* Construct the path of the file that will give us the DT location */
> + path = g_strdup_printf("/sys/bus/pci/devices/%s/devspec", host);
> + g_free(host);
> + if (!path || !g_file_get_contents(path, &buf, NULL, NULL)) {
> + goto err_out;
> + }
> + g_free(path);
Here we create a 'path' string, use it as the argument to
g_file_get_contents() and then free it (either here or in the err_out path)...
> +
> + /* Construct and read from host device tree the loc-code */
> + path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> + g_free(buf);
> + if (!path || !g_file_get_contents(path, &buf, NULL, NULL)) {
> + goto err_out;
> + }
> + return buf;
...but here we forget to free it before returning in the success case.
> +
> +err_out:
> + g_free(path);
> + return NULL;
> +}
Cleanest fix would be to declare 'path' and 'host' as
g_autofree char *path = NULL;
g_autofree char *host = NULL;
and then you can remove all the manual g_free(path) and g_free(host) calls.
thanks
-- PMM
