Re: [PATCH 4/8] ALSA: Avoid using timespec for struct snd_pcm_status

From: Arnd Bergmann
Date: Thu Apr 26 2018 - 08:50:34 EST


On Thu, Apr 26, 2018 at 12:53 PM, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:

>>
>> 971
>> 972 static int snd_pcm_status_user32(struct snd_pcm_substream *substream,
>> 973 struct snd_pcm_status32 __user * _status,
>> 974 bool ext)
>> 975 {
>> 976 struct snd_pcm_status64 status64;
>> 977 struct snd_pcm_status32 status32;
>> 978 int res;
>> 979
>> 980 memset(&status64, 0, sizeof(status64));
>> 981 memset(&status32, 0, sizeof(status32));
>> 982 /*
>> 983 * with extension, parameters are read/write,
>> 984 * get audio_tstamp_data from user,
>> 985 * ignore rest of status structure
>> 986 */
>> 987 if (ext && get_user(status64.audio_tstamp_data,
>> 988 (u32 __user *)(&_status->audio_tstamp_data)))
>> 989 return -EFAULT;
>> 990 res = snd_pcm_status64(substream, &status64);
>> 991 if (res < 0)
>> 992 return res;
>> 993
>> 994 status32 = (struct snd_pcm_status32) {
>> > 995 .state = status64.state,
>> 996 .trigger_tstamp_sec = status64.trigger_tstamp_sec,
>> 997 .trigger_tstamp_nsec = status64.trigger_tstamp_nsec,
>> 998 .tstamp_sec = status64.tstamp_sec,
>> 999 .tstamp_nsec = status64.tstamp_nsec,
>> 1000 .appl_ptr = status64.appl_ptr,
>> 1001 .hw_ptr = status64.hw_ptr,
>> 1002 .delay = status64.delay,
>> 1003 .avail = status64.avail,
>> 1004 .avail_max = status64.avail_max,
>> 1005 .overrange = status64.overrange,
>>> 1006 .suspended_state = status64.suspended_state,
>
> I am not sure for the warning here, we should change 'snd_pcm_state_t'
> to 's32' for struct snd_pcm_status64?
>
> typedef int __bitwise snd_pcm_state_t;

The problem is that snd_pcm_status32 uses 'u32' here instead of snd_pcm_state_t,
and the __bitwise annotation makes the two types incompatible. This is a
preexisting problem, the warning mail just appeared because you moved that
code to a different file.

If you want to avoid that warning, either use a type case with '__force',
or change the snd_pcm_status32 structure to also use snd_pcm_state_t.

That would be a useful change, but it should be separate from your
other changes since it's an unrelated problem.

Arnd