[PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for gmidi_in_port elements

From: Michal Nazarewicz
Date: Fri Jan 08 2016 - 22:48:09 EST


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@xxxxxxxxxx>
---
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