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

On Wed, 2016-03-09 at 11:23 -0500, Jes Sorensen wrote:

quoted

NeilBrown [off-list ref] writes:

quoted

On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:
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' ??

Seems reasonable - I'll put that in a separate patch.

Cheers,
Jes

Hello,
Thanks for the fix Jes.

I thinks that instead of making it "void" we can actually check the
return value and print a proper message if something goes wrong.
Also "simplified for() loop" can be applied to ahci_enumerate_ports() as well.

@@ -1624,7 +1624,10 @@ static int ahci_enumerate_ports(const char

*hba_path, int port_count, int host_b
 	 * this hba
 	 */
 	dir = opendir("/sys/dev/block");
-	for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
+	if (!dir)
+		return 1;
+
+	for (ent = readdir(dir); ent; ent = readdir(dir)) {
 		int fd;
 		char model[64];
 		char vendor[64];

@@ -2021,7 +2024,11 @@ static int detail_platform_imsm(int verbose,

int enumerate_only, char *controlle
 					print_imsm_capability(&entry->orom);
 					printf(" I/O Controller : %s (%s)\n",
  vmd_domain_to_controller(hba, buf), get_sys_dev_type(hba->type));
-					print_vmd_attached_devs(hba);
+					if (print_vmd_attached_devs(hba)) {
+						if (verbose > 0)
+ pr_err("failed to get devices attached to VMD domain.\n");
+						result |= 2;
+					}
 					printf("\n");
 				}
 			}

Is that ok with you? Should I prepare a proper patch with this?