USB: quirks: simplify quirk handling.

Commit Message

Pavel Machek Sept. 21, 2020, 11:30 a.m. UTC

Simplify quirk handling.

Signed-off-by: Pavel Machek (CIP) <[e-mail]>

Comments

Greg Kroah-Hartman Sept. 21, 2020, 11:38 a.m. UTC | #1

On Mon, Sep 21, 2020 at 01:30:39PM +0200, Pavel Machek wrote:
> Simplify quirk handling.

In what way?

Please be more descriptive in your changelog and resend.

thanks,

greg k-h

Pavel Machek Sept. 21, 2020, 11:47 a.m. UTC | #2

On Mon 2020-09-21 13:38:00, Greg KH wrote:
> On Mon, Sep 21, 2020 at 01:30:39PM +0200, Pavel Machek wrote:
> > Simplify quirk handling.
> 
> In what way?
> 
> Please be more descriptive in your changelog and resend.

Have you looked at the patch below? Please apply the patch. You are
free to edit the changelog if you feel the need.

								Pavel

Greg Kroah-Hartman Sept. 21, 2020, 11:52 a.m. UTC | #3

On Mon, Sep 21, 2020 at 01:47:29PM +0200, Pavel Machek wrote:
> On Mon 2020-09-21 13:38:00, Greg KH wrote:
> > On Mon, Sep 21, 2020 at 01:30:39PM +0200, Pavel Machek wrote:
> > > Simplify quirk handling.
> > 
> > In what way?
> > 
> > Please be more descriptive in your changelog and resend.
> 
> Have you looked at the patch below? Please apply the patch. You are
> free to edit the changelog if you feel the need.

I am also free to ignore patches with changelogs that do not say
anything of consequence, which I will do here.

greg k-h

Kars Mulder Sept. 23, 2020, 4:28 p.m. UTC | #4

On Monday, September 21, 2020 13:30 CEST, Pavel Machek <[e-mail]> wrote: 
> Simplify quirk handling.

This patch seems to contain two different "simplifications" in one.
I have no objections against the first simplification:

-	if (quirk_list) {
-		kfree(quirk_list);
-		quirk_list = NULL;
-	}
-
+	kfree(quirk_list);
 	quirk_list = kcalloc(quirk_count, sizeof(struct quirk_entry),
 			     GFP_KERNEL);

Since kfree() does nothing to nullpointers, all lines that are cut seem
to be superfluous.

The second simplification does not seem to introduce any new bugs as far
as I can tell. Due to lack of experience, I shall refrain from commenting
on whether or not it simplifies things.

Patch

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index f232914de5fd..167b6ac428a3 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -56,18 +56,13 @@  static int quirks_param_set(const char *value, const struct kernel_param *kp)
 		if (val[i] == ',')
 			quirk_count++;
 
-	if (quirk_list) {
-		kfree(quirk_list);
-		quirk_list = NULL;
-	}
-
+	kfree(quirk_list);
 	quirk_list = kcalloc(quirk_count, sizeof(struct quirk_entry),
 			     GFP_KERNEL);
 	if (!quirk_list) {
 		quirk_count = 0;
-		mutex_unlock(&quirk_mutex);
-		kfree(val);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto unlock;
 	}
 
 	for (i = 0, p = val; p && *p;) {
@@ -153,7 +148,7 @@  static int quirks_param_set(const char *value, const struct kernel_param *kp)
 	mutex_unlock(&quirk_mutex);
 	kfree(val);
 
-	return 0;
+	return err;
 }
 
 static const struct kernel_param_ops quirks_param_ops = {