Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 29 May 2019 21:15:32 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Simon =?iso-8859-1?Q?Sandstr=F6m?= <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
Message-ID: <20190529181532.GM24680@kadam>
References: <20190524110802.2953-1-simon@nikanor.nu>
 <20190524110802.2953-4-simon@nikanor.nu>
 <20190527073159.GX31203@kadam>
 <20190529155419.ego3sfedew65ini5@dev.nikanor.nu>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190529155419.ego3sfedew65ini5@dev.nikanor.nu>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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