All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: "Simon Sandström" <simon@nikanor.nu>
Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] staging: kpc2000: add missing spaces in core.c
Date: Wed, 29 May 2019 21:15:32 +0300	[thread overview]
Message-ID: <20190529181532.GM24680@kadam> (raw)
In-Reply-To: <20190529155419.ego3sfedew65ini5@dev.nikanor.nu>

On Wed, May 29, 2019 at 05:54:19PM +0200, Simon Sandström wrote:
> On Mon, May 27, 2019 at 10:31:59AM +0300, Dan Carpenter wrote:
> > On Fri, May 24, 2019 at 01:08:01PM +0200, Simon Sandström wrote:
> > > [..]
> > > -		ret = copy_to_user((void*)ioctl_param, (void*)&temp, sizeof(temp));
> > > +		ret = copy_to_user((void *)ioctl_param, (void *)&temp, sizeof(temp));
> > >  		if (ret)
> > >  			return -EFAULT;
> > 
> > This should really be written like so:
> > 
> > 		if (copy_to_user((void __user *)ioctl_param, &temp,
> > 				 sizeof(temp)))
> > 			return -EFAULT;
> > 
> > temp is really the wrong name.  "temp" is for temperatures.  "tmp" means
> > temporary.  But also "tmp" is wrong here because it's not a temporary
> > variable.  It's better to call it "regs" here.
> > 
> > regards,
> > dan carpenter
> > 
> 
> I agree, but I don't think it fits within this patch. I can send a
> separate patch with this change.

You could send the other chunk as a separate patch, but I don't think it
makes sense to apply this chunk when really it just needs to be
re-written.

I normally don't complain too much about mechanical no-thought patches,
but in this case the function is very sub-par and should be re-written.

regards,
dan carpenter

  reply	other threads:[~2019-05-29 18:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
2019-05-30 21:05   ` Greg KH
2019-05-30 22:57     ` Matt Sickler
2019-05-31  0:11       ` Greg KH
2019-05-31 10:52         ` [PATCH 0/2] staging: kpc2000: replace per-card misc devices Jeremy Sowden
2019-05-31 10:52           ` [PATCH 1/2] staging: kpc2000: export more device attributes via sysfs Jeremy Sowden
2019-06-03 12:08             ` Greg KH
2019-06-03 12:12               ` Greg KH
2019-05-31 10:52           ` [PATCH 2/2] staging: kpc2000: removed misc device Jeremy Sowden
2019-05-24 11:08 ` [PATCH 2/4] staging: kpc2000: remove extra blank line in core.c Simon Sandström
2019-05-24 11:08 ` [PATCH 3/4] staging: kpc2000: add missing spaces " Simon Sandström
2019-05-27  7:31   ` Dan Carpenter
2019-05-29 15:54     ` Simon Sandström
2019-05-29 18:15       ` Dan Carpenter [this message]
2019-05-24 11:08 ` [PATCH 4/4] staging: kpc2000: remove extra " Simon Sandström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190529181532.GM24680@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=simon@nikanor.nu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.