Messages in this thread | | | Subject | Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore | From | Kai Heng Feng <> | Date | Thu, 1 Mar 2018 00:01:40 +0800 |
| |
> On 28 Feb 2018, at 10:47 PM, Matthew Wilcox <willy@infradead.org> wrote: > > On Mon, Feb 26, 2018 at 11:04:57PM +0800, Kai-Heng Feng wrote: >> +static char quirks_param[128]; >> +module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644); >> +MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying >> quirks=vendorID:productID:quirks"); >> + >> +static char quirks_param_orig[128]; > >> +static u32 usb_detect_dynamic_quirks(struct usb_device *udev) >> +{ >> + u16 vid = le16_to_cpu(udev->descriptor.idVendor); >> + u16 pid = le16_to_cpu(udev->descriptor.idProduct); >> + struct quirk_entry *quirk; >> + >> + mutex_lock(&quirk_mutex); >> + if (strcmp(quirks_param, quirks_param_orig) != 0) { >> + strcpy(quirks_param_orig, quirks_param); > > What happens if the user is writing to quirks_param at the same time > that you memcpy it? > > I think you're going about this wrong by trying to use the > module_param_string machinery. You should be using module_param_cb() > to build the quirks list when the user writes it (and then translate > back into a string when the user wants to read from it.
Thanks! module_param_cb() is exactly what I want. I’ll use it in next version.
> Also, you won't need to use a linked list for this; you can just allocate > an array of quirks. >
I use linked list because the total quirks number is known after the entire string gets parsed. Do you suggest that I should just alloc a predefined number (like 16) quirk entries, instead of doing it dynamically?
Kai-Heng
| |