Re: [PATCH 5/8] {platform,super}-intel: Fix two resource leaks

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> The code did not free 'dir' allocated by opendir(). An additional
> benefit is that this simplifies the for() loops.
>
> Fixes: 60f0f54d ("IMSM: Add support for VMD")
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  platform-intel.c | 7 ++++++-
>  super-intel.c    | 6 +++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/platform-intel.c b/platform-intel.c
> index 88818f3..c60fd9e 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -724,8 +724,10 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
>  		return NULL;
>  
>  	dir = opendir("/sys/bus/pci/drivers/vmd");
> +	if (!dir)
> +		return NULL;
>  
> -	for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
> +	for (ent = readdir(dir); ent; ent = readdir(dir)) {
>  		sprintf(path, "/sys/bus/pci/drivers/vmd/%s/domain/device",
>  			ent->d_name);
>  
> @@ -734,8 +736,11 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
>  
>  		if (strncmp(buf, hba->path, strlen(buf)) == 0) {
>  			sprintf(path, "/sys/bus/pci/drivers/vmd/%s", ent->d_name);
> +			closedir(dir);
>  			return realpath(path, buf);
>  		}
>  	}
> +
> +	closedir(dir);
>  	return NULL;
>  }
> diff --git a/super-intel.c b/super-intel.c
> index 158f4e8..e1bee75 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1781,7 +1781,10 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
>  	 * this hba
>  	 */
>  	dir = opendir("/sys/bus/pci/drivers/nvme");
> -	for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
> +	if (!dir)
> +		return 1;
> +

Returning '1' looks really weird here.  I can see it is consistent with
	if (hba->type != SYS_DEV_VMD)
		return 1;

above, but still....
As the return value is never used, should we just make it 'void' ??

Thanks,
NeilBrown

> +	for (ent = readdir(dir); ent; ent = readdir(dir)) {
>  		int n;
>  
>  		/* is 'ent' a device? check that the 'subsystem' link exists and
> @@ -1814,6 +1817,7 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
>  		free(rp);
>  	}
>  
> +	closedir(dir);
>  	return 0;
>  }
>  
> -- 
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]