Re: [patch] copy_to_user check and whitespace cleanups in fs/cifs/file.c

[Jesper Juhl <juhl-lkml@dif.dk>]

On Sun, 26 Dec 2004, Alan Cox wrote:

> On Sul, 2004-12-26 at 23:24, Jesper Juhl wrote:
> > Hi,
> > 
> > Patch below adds a check for the copy_to_user return value and makes a few 
> > whitespace cleanups in  fs/cifs/file.c::cifs_user_read()
> > I hope bundling two different things together in one patch is OK when the 
> > change is as small as this, but if you want it spplit in two patches, then 
> > just say so.
> 
> Corrupts the stats
> Fails to free smb_read_data where in some cases it was freed before
> 
Whoops, I'll take care of that - thanks for pointing that out.

> I'm not sure the stats matter but I think you need something more like
> 
> 
> residue = copy_to_user(....)
> if(smb_read_data) {
>    cifs_buf_release(...)
>   ...
> }
> 
> Then
> 
> if(residue) {
>     total_read += bytes_read - residue;
>     FreeXid(xid);
>     return total_read ? total_read: -EFAULT;
> }
> 
> 

I take that to mean that if we do manage to read something before we run 
into the fault it's better to return what we have read than the fact that 
we ran into a fault... that makes sense. Sure, I'll cook up a patch to do 
that instead (tomorrow - I need some sleep).


-- 
Jesper Juhl