Re: [PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
On Sun, 2014-08-31 at 13:02 -0700, Greg Kroah-Hartman wrote: > Adding Alan Cox, as he pushed this driver upstream... It's sort of a false positive. The existing code will work fine. Arguably all of this should be using the dma_ APIs. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
On Mon, Sep 01, 2014 at 12:35:14PM +0300, Dan Carpenter wrote: > On Sun, Aug 31, 2014 at 09:46:22PM +0530, Sudip Mukherjee wrote: > > @@ -133,9 +134,15 @@ static ssize_t goldfish_audio_read(struct file *fp, > > char __user *buf, > > AUDIO_READ_BUFFER_AVAILABLE); > > > > /* copy data to user space */ > > - if (copy_to_user(buf, data->read_buffer, length)) > > + buffer = kzalloc(length, GFP_KERNEL); > > + if (buffer == NULL) > > + return -ENOMEM; > > + memcpy_fromio(buffer, data->read_buffer, length); > > + if (copy_to_user((void __user *)buf, buffer, length)) { > ^^^ > This cast shouldn't be needed. > > regards, > dan carpenter > yes , its not needed . I just checked it again after removing the cast. Actually I gave it as sparse was showing warning for argument 1 also. I should have rechecked it after fixing the warning for argument2. Sorry for it. Anyways, if Alan confirms that this patch is actually needed then I will resubmit a modified patch. thanks Sudip -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
On Sun, Aug 31, 2014 at 09:46:22PM +0530, Sudip Mukherjee wrote: > @@ -133,9 +134,15 @@ static ssize_t goldfish_audio_read(struct file *fp, char > __user *buf, > AUDIO_READ_BUFFER_AVAILABLE); > > /* copy data to user space */ > - if (copy_to_user(buf, data->read_buffer, length)) > + buffer = kzalloc(length, GFP_KERNEL); > + if (buffer == NULL) > + return -ENOMEM; > + memcpy_fromio(buffer, data->read_buffer, length); > + if (copy_to_user((void __user *)buf, buffer, length)) { ^^^ This cast shouldn't be needed. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
Adding Alan Cox, as he pushed this driver upstream... On Sun, Aug 31, 2014 at 09:46:22PM +0530, Sudip Mukherjee wrote: > fixed sparse warning of incorrect type in argument 1 and incorrect type in > argument 2 > it was directly dereferencing a __iomem pointer , which will work in x86 but > will fail in other architectures. Please wrap your changelog comments at 72 columns... > Signed-off-by: Sudip Mukherjee > --- > > hi, > can you please reveiew the patch and check if the approach is correct. If it > is correct then I can send another patch to correct the other similar > warnings present in this file. I don't think this is correct, because: > > drivers/staging/goldfish/goldfish_audio.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/goldfish/goldfish_audio.c > b/drivers/staging/goldfish/goldfish_audio.c > index c89d0b8..f9a13e7 100644 > --- a/drivers/staging/goldfish/goldfish_audio.c > +++ b/drivers/staging/goldfish/goldfish_audio.c > @@ -118,6 +118,7 @@ static ssize_t goldfish_audio_read(struct file *fp, char > __user *buf, > struct goldfish_audio *data = fp->private_data; > int length; > int result = 0; > + void *buffer; > > if (!data->read_supported) > return -ENODEV; > @@ -133,9 +134,15 @@ static ssize_t goldfish_audio_read(struct file *fp, char > __user *buf, > AUDIO_READ_BUFFER_AVAILABLE); > > /* copy data to user space */ > - if (copy_to_user(buf, data->read_buffer, length)) > + buffer = kzalloc(length, GFP_KERNEL); > + if (buffer == NULL) > + return -ENOMEM; > + memcpy_fromio(buffer, data->read_buffer, length); > + if (copy_to_user((void __user *)buf, buffer, length)) { > + kfree(buffer); > return -EFAULT; > - > + } > + kfree(buffer); The goldfish platform is an "emulator" so iomem is (or should be?) the same as "normal" memory here. So I think this is a false-positive. But I could be totally wrong here, Alan? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
fixed sparse warning of incorrect type in argument 1 and incorrect type in argument 2 it was directly dereferencing a __iomem pointer , which will work in x86 but will fail in other architectures. Signed-off-by: Sudip Mukherjee --- hi, can you please reveiew the patch and check if the approach is correct. If it is correct then I can send another patch to correct the other similar warnings present in this file. drivers/staging/goldfish/goldfish_audio.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/staging/goldfish/goldfish_audio.c b/drivers/staging/goldfish/goldfish_audio.c index c89d0b8..f9a13e7 100644 --- a/drivers/staging/goldfish/goldfish_audio.c +++ b/drivers/staging/goldfish/goldfish_audio.c @@ -118,6 +118,7 @@ static ssize_t goldfish_audio_read(struct file *fp, char __user *buf, struct goldfish_audio *data = fp->private_data; int length; int result = 0; + void *buffer; if (!data->read_supported) return -ENODEV; @@ -133,9 +134,15 @@ static ssize_t goldfish_audio_read(struct file *fp, char __user *buf, AUDIO_READ_BUFFER_AVAILABLE); /* copy data to user space */ - if (copy_to_user(buf, data->read_buffer, length)) + buffer = kzalloc(length, GFP_KERNEL); + if (buffer == NULL) + return -ENOMEM; + memcpy_fromio(buffer, data->read_buffer, length); + if (copy_to_user((void __user *)buf, buffer, length)) { + kfree(buffer); return -EFAULT; - + } + kfree(buffer); result += length; buf += length; count -= length; -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/