On Thu, Oct 21, 2010 at 07:45:45PM +0530, Koul, Vinod wrote: > > case _IOC_NR(SNDRV_SST_STREAM_SET_PARAMS): { > > - struct snd_sst_params *str_param = (struct snd_sst_params *)arg; > > + struct snd_sst_params str_param; > Now this is a struct... > > > [snip] > > + > > if (!str_id) { > So this becomes bogus check, can be removed :-) The check for str_id? That comes from near the start of the function and it's still needed. > > > > + retval = copy_to_user(dest, &retval, sizeof(__u32)); > > if (retval) > > retval = -EFAULT; > Trivial but this can be changed to > if (copy_to_user()) > retval = -EFAULT; > retval is the str_id here so you'd need an else statement as well. else retval = 0; I can send a separate patch for this. > Overall its good, tested it as well, Thanks for that. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html