Page MenuHomeFreeBSD
Differential D54130 Diff 169511 sys/dev/sound/midi/midi.c

Changeset View

Standalone View

View Options

sys/dev/sound/midi/midi.c

Show First 20 Lines • Show All 385 Lines • ▼ Show 20 Lines while (uio->uio_resid > 0) {
/* /*
* At this point, it is certain that m->inq has data * At this point, it is certain that m->inq has data
*/ */
used = MIN(MIDIQ_LEN(m->inq), uio->uio_resid); used = MIN(MIDIQ_LEN(m->inq), uio->uio_resid);
used = MIN(used, MIDI_RSIZE); used = MIN(used, MIDI_RSIZE);
MIDIQ_DEQ(m->inq, buf, used); MIDIQ_DEQ(m->inq, buf, used);
mtx_unlock(&m->lock);
retval = uiomove(buf, used, uio); retval = uiomove(buf, used, uio);
if (retval) if (retval)
goto err1; goto err0;
mtx_lock(&m->lock);
emasteUnsubmitted
Not Done
Inline Actions

are there any struct snd_midimembers that could be invalid after dropping and reacquiring the lock? E.g. could m->flags & M_RX no longer be true?

emaste: are there any `struct snd_midi`members that could be invalid after dropping and reacquiring the…
christosAuthorUnsubmitted
Done
Inline Actions

The only place where M_RX gets cleared is in midi_close(), so I'm not sure we should worry about this, since by that time we'll have closed and midi_read() will just need to flush. Or am I missing something?

christos: The only place where `M_RX` gets cleared is in `midi_close()`, so I'm not sure we should worry…
} }
/* /*
* If we Made it here then transfer is good * If we Made it here then transfer is good
*/ */
retval = 0; retval = 0;
err1: err1:
mtx_unlock(&m->lock); mtx_unlock(&m->lock);
▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Lines while (uio->uio_resid > 0) {
/* /*
* We are certain than data can be placed on the queue * We are certain than data can be placed on the queue
*/ */
used = MIN(MIDIQ_AVAIL(m->outq), uio->uio_resid); used = MIN(MIDIQ_AVAIL(m->outq), uio->uio_resid);
used = MIN(used, MIDI_WSIZE); used = MIN(used, MIDI_WSIZE);
mtx_unlock(&m->lock);
retval = uiomove(buf, used, uio); retval = uiomove(buf, used, uio);
if (retval) if (retval)
goto err1; goto err0;
mtx_lock(&m->lock);
MIDIQ_ENQ(m->outq, buf, used); MIDIQ_ENQ(m->outq, buf, used);
markjUnsubmitted
Not Done
Inline Actions

used is calculated above: used = MIN(MIDIQ_AVAIL(m->outq), uio->uio_resid). What if the amount of available space changed while the lock was dropped?

markj: `used` is calculated above: `used = MIN(MIDIQ_AVAIL(m->outq), uio->uio_resid)`. What if the…
christosAuthorUnsubmitted
Done
Inline Actions

I'm not really sure how to address this. I suppose if the available size since then has changed to something less than used, I suppose we will discard a few bytes. Shouldn't that be the normal case though? We've got a statically-sized buffer,so if some other thread fills it first, then we won't have available space until we drain it.

christos: I'm not really sure how to address this. I suppose if the available size since then has changed…
christosAuthorUnsubmitted
Done
Inline Actions

@markj Bump ^

christos: @markj Bump ^
markjUnsubmitted
Not Done
Inline Actions

The MIDIQ_ENQ macro has a warning comment: "No protection against overflow, underflow". So is it actually safe to write more bytes than there is space available? That is, will doing so crash the system? If not, what happens? Will we just drop user data?

Looking at the code, I think we are just dropping data. I presume that is ok in this context.

markj: The MIDIQ_ENQ macro has a warning comment: "No protection against overflow, underflow". So is…
/* /*
* Inform the bottom half that data can be written * Inform the bottom half that data can be written
*/ */
if (!(m->flags & M_TXEN)) { if (!(m->flags & M_TXEN)) {
m->flags |= M_TXEN; m->flags |= M_TXEN;
MPU_CALLBACK(m, m->cookie, m->flags); MPU_CALLBACK(m, m->cookie, m->flags);
} }
} }
▲ Show 20 Lines • Show All 64 Lines • Show Last 20 Lines