Dan Carpenter schrieb:
> copy_to_user() returns the number of bytes remaining to be copied which
> isn't the right thing to return here. The comments say that these
> functions in dvb_ca_en50221.c should return the number of bytes copied or
> an error return. I've changed it to return -EFAULT.
>
> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
>
> diff --git a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> index ef259a0..aa7a298 100644
> --- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> @@ -1318,8 +1318,10 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
>
> fragbuf[0] = connection_id;
> fragbuf[1] = ((fragpos + fraglen) < count) ? 0x80 : 0x00;
> - if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
> + if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0) {
> + status = -EFAULT;
> goto exit;
> + }
>
> timeout = jiffies + HZ / 2;
> written = 0;
> @@ -1494,8 +1496,10 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user * buf,
>
> hdr[0] = slot;
> hdr[1] = connection_id;
> - if ((status = copy_to_user(buf, hdr, 2)) != 0)
> + if ((status = copy_to_user(buf, hdr, 2)) != 0) {
> + status = -EFAULT;
> goto exit;
> + }
> status = pktlen;
>
> exit:
> --
Doint to many things at once is bad. IMHO it is more readable to do so:
+status = copy_to_user(buf, hdr, 2);
+if ( status != 0) {
Maybe the maintainer has different ideas but especialy lines like will gain.
-if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
+status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen):
+if ( status != 0) {
just my 2 cents,
wh
--
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
