All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jes Sorensen <Jes.Sorensen@redhat.com>
To: "Baldysiak, Pawel" <pawel.baldysiak@intel.com>
Cc: "neilb@suse.de" <neilb@suse.de>,
	"linux-raid@vger.kernel.org" <linux-raid@vger.kernel.org>,
	"gqjiang@suse.com" <gqjiang@suse.com>
Subject: Re: [PATCH 5/8] {platform,super}-intel: Fix two resource leaks
Date: Thu, 10 Mar 2016 11:37:26 -0500	[thread overview]
Message-ID: <wrfjr3fi1e7t.fsf@redhat.com> (raw)
In-Reply-To: <1457608450.28311.10.camel@intel.com> (Pawel Baldysiak's message of "Thu, 10 Mar 2016 11:14:10 +0000")

"Baldysiak, Pawel" <pawel.baldysiak@intel.com> writes:
> On Wed, 2016-03-09 at 11:23 -0500, Jes Sorensen wrote:
>> NeilBrown <neilb@suse.de> writes:
>> > 
>> > 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?

Hi Pawel,

I already pushed this into git, but if you want to send me a patch (or
two) for this, I am happy to apply them. Lets make the
ahci_enumerate_ports() change in a separate patch.

Cheers,
Jes
--
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

  reply	other threads:[~2016-03-10 16:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-08 17:30 [PATCH 0/8] mdadm static checker fixes Jes.Sorensen
2016-03-08 17:30 ` [PATCH 1/8] Manage: Manage_add(): Fix memory leak Jes.Sorensen
2016-03-08 17:30 ` [PATCH 2/8] load_sys(): Add a buffer size argument Jes.Sorensen
2016-03-08 17:30 ` [PATCH 3/8] Grow: Grow_continue_command() remove dead code Jes.Sorensen
2016-03-08 22:42   ` NeilBrown
2016-03-09 16:19     ` Jes Sorensen
2016-03-08 17:30 ` [PATCH 4/8] Grow: Grow_addbitmap(): Add check to quiet down static code checkers Jes.Sorensen
2016-03-09 17:42   ` Guoqing Jiang
2016-03-09 14:00     ` Jes Sorensen
2016-03-10  7:21       ` NeilBrown
2016-03-10 16:40         ` Jes Sorensen
2016-03-08 17:30 ` [PATCH 5/8] {platform,super}-intel: Fix two resource leaks Jes.Sorensen
2016-03-08 22:45   ` NeilBrown
2016-03-09 16:23     ` Jes Sorensen
2016-03-10 11:14       ` Baldysiak, Pawel
2016-03-10 16:37         ` Jes Sorensen [this message]
2016-03-08 17:30 ` [PATCH 6/8] bitmap: Fix resource leak in bitmap_file_open() Jes.Sorensen
2016-03-08 22:50   ` NeilBrown
2016-03-09 16:28     ` Jes Sorensen
2016-03-08 17:30 ` [PATCH 7/8] Manage: Manage_subdevs() fix file descriptor leak Jes.Sorensen
2016-03-08 17:30 ` [PATCH 8/8] super1: Fix potential buffer overflows when copying cluster_name Jes.Sorensen
2016-03-08 22:55 ` [PATCH 0/8] mdadm static checker fixes NeilBrown
2016-03-09 16:30   ` Jes Sorensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=wrfjr3fi1e7t.fsf@redhat.com \
    --to=jes.sorensen@redhat.com \
    --cc=gqjiang@suse.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=pawel.baldysiak@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.