* [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements
@ 2016-01-09 3:47 Michal Nazarewicz
2016-01-09 3:47 ` [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path Michal Nazarewicz
2016-01-13 18:26 ` [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Felipe Ferreri Tonello
0 siblings, 2 replies; 7+ messages in thread
From: Michal Nazarewicz @ 2016-01-09 3:47 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, Dan Carpenter
Cc: Felipe F. Tonello, Robert Baldyga, Andrzej Pietrasiewicz,
Pawel Szewczyk, linux-kernel, Michal Nazarewicz
Reduce number of allocations, simplify memory management and reduce
memory usage by stacking the gmidi_in_port elements at the end of the
f_midi structure using a flexible array.
Also, observe that gmidi_in_port::midi pointer is *never* used for any
purpose so it can be safely removed.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_midi.c | 43 ++++++++++++------------------------
1 file changed, 14 insertions(+), 29 deletions(-)
On Tue, Jan 05 2016, Dan Carpenter wrote:
> On Tue, Jan 05, 2016 at 02:55:31PM +0100, Michal Nazarewicz wrote:
>> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
>> {
>> struct f_midi *midi = substream->rmidi->private_data;
>>
>> - if (!midi->in_port[substream->number])
>> + if (substream->number > midi->in_ports)
>
> This is off by one. It should be >= midi->in_ports.
Fixed.
Since there were no objections to this small patch set (i.e. there was
mostly silence about it), I’ll go ahead and interpret it as ‘go ahead
and properly submit it’. :)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 898a570..fd67af2 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
* USB <- IN endpoint <- rawmidi
*/
struct gmidi_in_port {
- struct f_midi *midi;
int active;
uint8_t cable;
uint8_t state;
@@ -78,7 +77,6 @@ struct f_midi {
struct snd_rawmidi_substream *in_substream[MAX_PORTS];
struct snd_rawmidi_substream *out_substream[MAX_PORTS];
- struct gmidi_in_port *in_port[MAX_PORTS];
unsigned long out_triggered;
struct tasklet_struct tasklet;
@@ -87,6 +85,8 @@ struct f_midi {
int index;
char *id;
unsigned int buflen, qlen;
+
+ struct gmidi_in_port in_ports_array[/* in_ports */];
};
static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -529,11 +529,11 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
req->length = 0;
req->complete = f_midi_complete;
- for (i = 0; i < MAX_PORTS; i++) {
- struct gmidi_in_port *port = midi->in_port[i];
+ for (i = 0; i < midi->in_ports; i++) {
+ struct gmidi_in_port *port = midi->in_ports_array + i;
struct snd_rawmidi_substream *substream = midi->in_substream[i];
- if (!port || !port->active || !substream)
+ if (!port->active || !substream)
continue;
while (req->length + 3 < midi->buflen) {
@@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
{
struct f_midi *midi = substream->rmidi->private_data;
- if (!midi->in_port[substream->number])
+ if (substream->number >= midi->in_ports)
return -EINVAL;
VDBG(midi, "%s()\n", __func__);
midi->in_substream[substream->number] = substream;
- midi->in_port[substream->number]->state = STATE_UNKNOWN;
+ midi->in_ports_array[substream->number].state = STATE_UNKNOWN;
return 0;
}
@@ -589,11 +589,11 @@ static void f_midi_in_trigger(struct snd_rawmidi_substream *substream, int up)
{
struct f_midi *midi = substream->rmidi->private_data;
- if (!midi->in_port[substream->number])
+ if (substream->number >= midi->in_ports)
return;
VDBG(midi, "%s() %d\n", __func__, up);
- midi->in_port[substream->number]->active = up;
+ midi->in_ports_array[substream->number].active = up;
if (up)
tasklet_hi_schedule(&midi->tasklet);
}
@@ -1067,14 +1067,11 @@ static void f_midi_free(struct usb_function *f)
{
struct f_midi *midi;
struct f_midi_opts *opts;
- int i;
midi = func_to_midi(f);
opts = container_of(f->fi, struct f_midi_opts, func_inst);
kfree(midi->id);
mutex_lock(&opts->lock);
- for (i = opts->in_ports - 1; i >= 0; --i)
- kfree(midi->in_port[i]);
kfree(midi);
--opts->refcnt;
mutex_unlock(&opts->lock);
@@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
}
/* allocate and initialize one new instance */
- midi = kzalloc(sizeof(*midi), GFP_KERNEL);
+ midi = kzalloc(
+ sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
+ GFP_KERNEL);
if (!midi) {
mutex_unlock(&opts->lock);
return ERR_PTR(-ENOMEM);
}
- for (i = 0; i < opts->in_ports; i++) {
- struct gmidi_in_port *port = kzalloc(sizeof(*port), GFP_KERNEL);
-
- if (!port) {
- status = -ENOMEM;
- mutex_unlock(&opts->lock);
- goto setup_fail;
- }
-
- port->midi = midi;
- port->active = 0;
- port->cable = i;
- midi->in_port[i] = port;
- }
+ for (i = 0; i < opts->in_ports; i++)
+ midi->in_ports_array[i].cable = i;
/* set up ALSA midi devices */
midi->id = kstrdup(opts->id, GFP_KERNEL);
@@ -1161,8 +1148,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
return &midi->func;
setup_fail:
- for (--i; i >= 0; i--)
- kfree(midi->in_port[i]);
kfree(midi);
return ERR_PTR(status);
}
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path
2016-01-09 3:47 [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Michal Nazarewicz
@ 2016-01-09 3:47 ` Michal Nazarewicz
2016-01-13 18:29 ` Felipe Ferreri Tonello
2016-01-13 18:26 ` [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Felipe Ferreri Tonello
1 sibling, 1 reply; 7+ messages in thread
From: Michal Nazarewicz @ 2016-01-09 3:47 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, Dan Carpenter
Cc: Felipe F. Tonello, Robert Baldyga, Andrzej Pietrasiewicz,
Pawel Szewczyk, linux-kernel, Michal Nazarewicz
From: Dan Carpenter <dan.carpenter@oracle.com>
We added a new error path to this function and we forgot to drop the
lock.
Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
[mina86@mina86.com: rebased on top of refactoring commit]
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_midi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index fd67af2..f287c68 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1098,7 +1098,7 @@ static void f_midi_unbind(struct usb_configuration *c, struct usb_function *f)
static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
{
- struct f_midi *midi;
+ struct f_midi *midi = NULL;
struct f_midi_opts *opts;
int status, i;
@@ -1107,8 +1107,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
mutex_lock(&opts->lock);
/* sanity check */
if (opts->in_ports > MAX_PORTS || opts->out_ports > MAX_PORTS) {
- mutex_unlock(&opts->lock);
- return ERR_PTR(-EINVAL);
+ status = -EINVAL;
+ goto setup_fail;
}
/* allocate and initialize one new instance */
@@ -1116,8 +1116,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
GFP_KERNEL);
if (!midi) {
- mutex_unlock(&opts->lock);
- return ERR_PTR(-ENOMEM);
+ status = -ENOMEM;
+ goto setup_fail;
}
for (i = 0; i < opts->in_ports; i++)
@@ -1127,7 +1127,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
midi->id = kstrdup(opts->id, GFP_KERNEL);
if (opts->id && !midi->id) {
status = -ENOMEM;
- mutex_unlock(&opts->lock);
goto setup_fail;
}
midi->in_ports = opts->in_ports;
@@ -1148,6 +1147,7 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
return &midi->func;
setup_fail:
+ mutex_unlock(&opts->lock);
kfree(midi);
return ERR_PTR(status);
}
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements
2016-01-09 3:47 [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Michal Nazarewicz
2016-01-09 3:47 ` [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path Michal Nazarewicz
@ 2016-01-13 18:26 ` Felipe Ferreri Tonello
2016-01-13 20:41 ` Dan Carpenter
2016-01-18 16:02 ` Michal Nazarewicz
1 sibling, 2 replies; 7+ messages in thread
From: Felipe Ferreri Tonello @ 2016-01-13 18:26 UTC (permalink / raw)
To: Michal Nazarewicz, Felipe Balbi, Greg Kroah-Hartman,
Dan Carpenter
Cc: Robert Baldyga, Andrzej Pietrasiewicz, Pawel Szewczyk,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5489 bytes --]
Hi Michael,
On 09/01/16 03:47, Michal Nazarewicz wrote:
> Reduce number of allocations, simplify memory management and reduce
> memory usage by stacking the gmidi_in_port elements at the end of the
> f_midi structure using a flexible array.
>
> Also, observe that gmidi_in_port::midi pointer is *never* used for any
> purpose so it can be safely removed.
>
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 43 ++++++++++++------------------------
> 1 file changed, 14 insertions(+), 29 deletions(-)
>
> On Tue, Jan 05 2016, Dan Carpenter wrote:
>> On Tue, Jan 05, 2016 at 02:55:31PM +0100, Michal Nazarewicz wrote:
>>> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
>>> {
>>> struct f_midi *midi = substream->rmidi->private_data;
>>>
>>> - if (!midi->in_port[substream->number])
>>> + if (substream->number > midi->in_ports)
>>
>> This is off by one. It should be >= midi->in_ports.
>
> Fixed.
>
> Since there were no objections to this small patch set (i.e. there was
> mostly silence about it), I’ll go ahead and interpret it as ‘go ahead
> and properly submit it’. :)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 898a570..fd67af2 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
> * USB <- IN endpoint <- rawmidi
> */
> struct gmidi_in_port {
> - struct f_midi *midi;
This change is unrelated. I sent a patch removing this pointer as well.
> int active;
> uint8_t cable;
> uint8_t state;
> @@ -78,7 +77,6 @@ struct f_midi {
>
> struct snd_rawmidi_substream *in_substream[MAX_PORTS];
> struct snd_rawmidi_substream *out_substream[MAX_PORTS];
> - struct gmidi_in_port *in_port[MAX_PORTS];
>
> unsigned long out_triggered;
> struct tasklet_struct tasklet;
> @@ -87,6 +85,8 @@ struct f_midi {
> int index;
> char *id;
> unsigned int buflen, qlen;
> +
> + struct gmidi_in_port in_ports_array[/* in_ports */];
> };
>
> static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -529,11 +529,11 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> req->length = 0;
> req->complete = f_midi_complete;
>
> - for (i = 0; i < MAX_PORTS; i++) {
> - struct gmidi_in_port *port = midi->in_port[i];
> + for (i = 0; i < midi->in_ports; i++) {
> + struct gmidi_in_port *port = midi->in_ports_array + i;
> struct snd_rawmidi_substream *substream = midi->in_substream[i];
>
> - if (!port || !port->active || !substream)
> + if (!port->active || !substream)
> continue;
>
> while (req->length + 3 < midi->buflen) {
> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
> {
> struct f_midi *midi = substream->rmidi->private_data;
>
> - if (!midi->in_port[substream->number])
> + if (substream->number >= midi->in_ports)
> return -EINVAL;
>
> VDBG(midi, "%s()\n", __func__);
> midi->in_substream[substream->number] = substream;
> - midi->in_port[substream->number]->state = STATE_UNKNOWN;
> + midi->in_ports_array[substream->number].state = STATE_UNKNOWN;
> return 0;
> }
>
> @@ -589,11 +589,11 @@ static void f_midi_in_trigger(struct snd_rawmidi_substream *substream, int up)
> {
> struct f_midi *midi = substream->rmidi->private_data;
>
> - if (!midi->in_port[substream->number])
> + if (substream->number >= midi->in_ports)
> return;
>
> VDBG(midi, "%s() %d\n", __func__, up);
> - midi->in_port[substream->number]->active = up;
> + midi->in_ports_array[substream->number].active = up;
> if (up)
> tasklet_hi_schedule(&midi->tasklet);
> }
> @@ -1067,14 +1067,11 @@ static void f_midi_free(struct usb_function *f)
> {
> struct f_midi *midi;
> struct f_midi_opts *opts;
> - int i;
>
> midi = func_to_midi(f);
> opts = container_of(f->fi, struct f_midi_opts, func_inst);
> kfree(midi->id);
> mutex_lock(&opts->lock);
> - for (i = opts->in_ports - 1; i >= 0; --i)
> - kfree(midi->in_port[i]);
> kfree(midi);
> --opts->refcnt;
> mutex_unlock(&opts->lock);
> @@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> }
>
> /* allocate and initialize one new instance */
> - midi = kzalloc(sizeof(*midi), GFP_KERNEL);
> + midi = kzalloc(
> + sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
> + GFP_KERNEL);
Is there a garantee that the compiler will always use in_ports_array at
the end of the allocated block?
> if (!midi) {
> mutex_unlock(&opts->lock);
> return ERR_PTR(-ENOMEM);
> }
>
> - for (i = 0; i < opts->in_ports; i++) {
> - struct gmidi_in_port *port = kzalloc(sizeof(*port), GFP_KERNEL);
> -
> - if (!port) {
> - status = -ENOMEM;
> - mutex_unlock(&opts->lock);
> - goto setup_fail;
> - }
> -
> - port->midi = midi;
> - port->active = 0;
> - port->cable = i;
> - midi->in_port[i] = port;
> - }
> + for (i = 0; i < opts->in_ports; i++)
> + midi->in_ports_array[i].cable = i;
>
> /* set up ALSA midi devices */
> midi->id = kstrdup(opts->id, GFP_KERNEL);
> @@ -1161,8 +1148,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> return &midi->func;
>
> setup_fail:
> - for (--i; i >= 0; i--)
> - kfree(midi->in_port[i]);
> kfree(midi);
> return ERR_PTR(status);
> }
>
Felipe
[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7310 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path
2016-01-09 3:47 ` [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path Michal Nazarewicz
@ 2016-01-13 18:29 ` Felipe Ferreri Tonello
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Ferreri Tonello @ 2016-01-13 18:29 UTC (permalink / raw)
To: Michal Nazarewicz, Felipe Balbi, Greg Kroah-Hartman,
Dan Carpenter
Cc: Robert Baldyga, Andrzej Pietrasiewicz, Pawel Szewczyk,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]
Hi Dan,
On 09/01/16 03:47, Michal Nazarewicz wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
>
> We added a new error path to this function and we forgot to drop the
> lock.
>
> Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> [mina86@mina86.com: rebased on top of refactoring commit]
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index fd67af2..f287c68 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1098,7 +1098,7 @@ static void f_midi_unbind(struct usb_configuration *c, struct usb_function *f)
>
> static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> {
> - struct f_midi *midi;
> + struct f_midi *midi = NULL;
> struct f_midi_opts *opts;
> int status, i;
>
> @@ -1107,8 +1107,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> mutex_lock(&opts->lock);
> /* sanity check */
> if (opts->in_ports > MAX_PORTS || opts->out_ports > MAX_PORTS) {
> - mutex_unlock(&opts->lock);
> - return ERR_PTR(-EINVAL);
> + status = -EINVAL;
> + goto setup_fail;
> }
>
> /* allocate and initialize one new instance */
> @@ -1116,8 +1116,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
> GFP_KERNEL);
> if (!midi) {
> - mutex_unlock(&opts->lock);
> - return ERR_PTR(-ENOMEM);
> + status = -ENOMEM;
> + goto setup_fail;
> }
>
> for (i = 0; i < opts->in_ports; i++)
> @@ -1127,7 +1127,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> midi->id = kstrdup(opts->id, GFP_KERNEL);
> if (opts->id && !midi->id) {
> status = -ENOMEM;
> - mutex_unlock(&opts->lock);
> goto setup_fail;
> }
> midi->in_ports = opts->in_ports;
> @@ -1148,6 +1147,7 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> return &midi->func;
>
> setup_fail:
> + mutex_unlock(&opts->lock);
> kfree(midi);
> return ERR_PTR(status);
> }
>
[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7195 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements
2016-01-13 18:26 ` [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Felipe Ferreri Tonello
@ 2016-01-13 20:41 ` Dan Carpenter
2016-01-18 16:02 ` Michal Nazarewicz
1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2016-01-13 20:41 UTC (permalink / raw)
To: Felipe Ferreri Tonello
Cc: Michal Nazarewicz, Felipe Balbi, Greg Kroah-Hartman,
Robert Baldyga, Andrzej Pietrasiewicz, Pawel Szewczyk,
linux-kernel
On Wed, Jan 13, 2016 at 06:26:22PM +0000, Felipe Ferreri Tonello wrote:
> > @@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> > }
> >
> > /* allocate and initialize one new instance */
> > - midi = kzalloc(sizeof(*midi), GFP_KERNEL);
> > + midi = kzalloc(
> > + sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
> > + GFP_KERNEL);
>
> Is there a garantee that the compiler will always use in_ports_array at
> the end of the allocated block?
>
Yes. It's fine. Zero size arrays at the end of a struct are normal.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements
2016-01-13 18:26 ` [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Felipe Ferreri Tonello
2016-01-13 20:41 ` Dan Carpenter
@ 2016-01-18 16:02 ` Michal Nazarewicz
2016-01-19 9:46 ` Felipe Ferreri Tonello
1 sibling, 1 reply; 7+ messages in thread
From: Michal Nazarewicz @ 2016-01-18 16:02 UTC (permalink / raw)
To: Felipe Ferreri Tonello, Felipe Balbi, Greg Kroah-Hartman,
Dan Carpenter
Cc: Robert Baldyga, Andrzej Pietrasiewicz, Pawel Szewczyk,
linux-kernel
> On 09/01/16 03:47, Michal Nazarewicz wrote:
>> @@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
>> * USB <- IN endpoint <- rawmidi
>> */
>> struct gmidi_in_port {
>> - struct f_midi *midi;
On Wed, Jan 13 2016, Felipe Ferreri Tonello wrote:
> This change is unrelated. I sent a patch removing this pointer as well.
Has it been merged yet? Which branch? Could you point me to the patch?
>> int active;
>> uint8_t cable;
>> uint8_t state;
>> @@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
>> }
>>
>> /* allocate and initialize one new instance */
>> - midi = kzalloc(sizeof(*midi), GFP_KERNEL);
>> + midi = kzalloc(
>> + sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
>> + GFP_KERNEL);
>
> Is there a garantee that the compiler will always use in_ports_array at
> the end of the allocated block?
Yep, see <https://en.wikipedia.org/wiki/Flexible_array_member>.
>
>> if (!midi) {
>> mutex_unlock(&opts->lock);
>> return ERR_PTR(-ENOMEM);
>> }
>>
>> - for (i = 0; i < opts->in_ports; i++) {
>> - struct gmidi_in_port *port = kzalloc(sizeof(*port), GFP_KERNEL);
>> -
>> - if (!port) {
>> - status = -ENOMEM;
>> - mutex_unlock(&opts->lock);
>> - goto setup_fail;
>> - }
>> -
>> - port->midi = midi;
>> - port->active = 0;
>> - port->cable = i;
>> - midi->in_port[i] = port;
>> - }
>> + for (i = 0; i < opts->in_ports; i++)
>> + midi->in_ports_array[i].cable = i;
>>
>> /* set up ALSA midi devices */
>> midi->id = kstrdup(opts->id, GFP_KERNEL);
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, ミハウ “mina86” ナザレヴイツ (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements
2016-01-18 16:02 ` Michal Nazarewicz
@ 2016-01-19 9:46 ` Felipe Ferreri Tonello
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Ferreri Tonello @ 2016-01-19 9:46 UTC (permalink / raw)
To: Michal Nazarewicz, Felipe Balbi, Greg Kroah-Hartman,
Dan Carpenter
Cc: Robert Baldyga, Andrzej Pietrasiewicz, Pawel Szewczyk,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]
Hi Michal,
On 18/01/16 16:02, Michal Nazarewicz wrote:
>> On 09/01/16 03:47, Michal Nazarewicz wrote:
>>> @@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
>>> * USB <- IN endpoint <- rawmidi
>>> */
>>> struct gmidi_in_port {
>>> - struct f_midi *midi;
>
> On Wed, Jan 13 2016, Felipe Ferreri Tonello wrote:
>> This change is unrelated. I sent a patch removing this pointer as well.
>
> Has it been merged yet? Which branch? Could you point me to the patch?
No, but it is in the mailing list.
"[PATCH 3/4] usb: gadget: f_midi: remove useless midi reference from
port struct" under "[PATCH 0/4] More improvements on MIDI gadget function"
>
>>> int active;
>>> uint8_t cable;
>>> uint8_t state;
>>> @@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
>>> }
>>>
>>> /* allocate and initialize one new instance */
>>> - midi = kzalloc(sizeof(*midi), GFP_KERNEL);
>>> + midi = kzalloc(
>>> + sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
>>> + GFP_KERNEL);
>>
>> Is there a garantee that the compiler will always use in_ports_array at
>> the end of the allocated block?
>
> Yep, see <https://en.wikipedia.org/wiki/Flexible_array_member>.
Thanks.
Felipe
[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7195 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-19 9:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-09 3:47 [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Michal Nazarewicz
2016-01-09 3:47 ` [PATCH 2/2] usb: gadget: f_midi: missing unlock on error path Michal Nazarewicz
2016-01-13 18:29 ` Felipe Ferreri Tonello
2016-01-13 18:26 ` [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements Felipe Ferreri Tonello
2016-01-13 20:41 ` Dan Carpenter
2016-01-18 16:02 ` Michal Nazarewicz
2016-01-19 9:46 ` Felipe Ferreri Tonello
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox