On Mon, Nov 07, 2022 at 02:32:44PM +0100, Theo Buehler wrote:
> On Mon, Nov 07, 2022 at 12:41:14PM +0000, Matthieu Herrb wrote:
>
> [...]
>
> > Fix a small leak in pci_system_openbsd_destroy() while there.
>
> Similar leaks exist in pci_system_openbsd_create() which I think should
> call pci_system_openbsd_destroy() in both early returns instead of
> duplicating it in the second one.
>
> pci_system_init() returning non-zero will hit an err(), so these leaks
> don't really matter.
>
> compile-tested only since I don't know how to exercise this code.
>
Hmm you made me look closer at the common code. for
pci_system_cleanup() which calls the OS specific destroy function
free(pci_sys) is done in the common code. So adding those in the OS
specific functions is dumb. (and I don't know why the libpciaccess
0.17 patch add them to freebsd...)
OTOH there no such code in pci_system_init() so your patch makes
sense, even though in most cases the caller will exit on failure
anyways.
> Index: src/openbsd_pci.c
> ===================================================================
> RCS file: /cvs/xenocara/lib/libpciaccess/src/openbsd_pci.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 openbsd_pci.c
> --- src/openbsd_pci.c 3 Sep 2021 07:19:13 -0000 1.28
> +++ src/openbsd_pci.c 7 Nov 2022 13:25:09 -0000
> @@ -338,6 +338,8 @@ pci_system_openbsd_destroy(void)
> for (domain = 0; domain < ndomains; domain++)
> close(pcifd[domain]);
> ndomains = 0;
> + free(pci_sys);
> + pci_sys = NULL;
> }
>
> static int
> @@ -651,8 +653,10 @@ pci_system_openbsd_create(void)
> ndomains++;
> }
>
> - if (ndomains == 0)
> + if (ndomains == 0) {
> + pci_system_openbsd_destroy();
> return ENXIO;
> + }
>
> ndevs = 0;
> for (domain = 0; domain < ndomains; domain++) {
> @@ -676,11 +680,7 @@ pci_system_openbsd_create(void)
> pci_sys->num_devices = ndevs;
> pci_sys->devices = calloc(ndevs, sizeof(struct pci_device_private));
> if (pci_sys->devices == NULL) {
> - free(pci_sys);
> - pci_sys = NULL;
> - for (domain = 0; domain < ndomains; domain++)
> - close(pcifd[domain]);
> - ndomains = 0;
> + pci_system_openbsd_destroy();
> return ENOMEM;
> }
>
>
--
Matthieu Herrb
