On 12/10/14, 11:28 AM, Takashi Iwai wrote:
At Mon, 8 Dec 2014 16:23:43 -0600, Pierre-Louis Bossart wrote:
Let userspace select audio timestamp config, ignore and zero all other fields
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/core/pcm_native.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 37a7137..7dcd6bd 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -706,6 +706,7 @@ int snd_pcm_status(struct snd_pcm_substream *substream, struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_stream_lock_irq(substream);
- memset(status, 0, sizeof(*status));
This memset() can be outside the lock.
status->state = runtime->status->state; status->suspended_state = runtime->status->suspended_state; if (status->state == SNDRV_PCM_STATE_OPEN) @@ -757,7 +758,8 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream, struct snd_pcm_status status; int res;
- memset(&status, 0, sizeof(status));
- if (copy_from_user(&status, _status, sizeof(status)))
return -EFAULT;You're clearing the data at the beginning of snd_pcm_status(), so it doesn't make sense to do copy_from_user().
In anyway, I prefer passing the extra argument for the parameter user may set instead of initializing the whole struct beforehand. snd_pcm_status() is called only internally, so it's no big problem to change the arguments.
Did you mean something like this, only read the relevant field from the larger structure?
if (copy_from_user(&audio_tstamp_data, _status.audio_tstamp_data, sizeof(int))) return -EFAULT; res = snd_pcm_status(substream, &status, audio_tstamp_data);