Re: [PATCH] ROMFS 0 byte file read error

[Chris Fester]

On Wed, Jul 30, 2008 at 11:06:29AM -0700, Linus Torvalds wrote:
> Something like this. UNTESTED. Pls verify and send back if this is ok. The 
> patch is certainly bigger, but I think that the end result is more 
> obvious.
> 
> 		Linus

I tested this patch by:
1.)  cat'ing the 0 length file.
2.)  doing a diff between the loop mounted filesystem and the source
directory for the romfs image.
3.)  cp -rdp'ing the contents of the loop mounted filesystem to another
directory, diffing that directory to the source directory.

All of those tests pass.  The image I'm testing with is about 23MB, with
900 files and directories.

I agree that your patch generates far more straightforward code than the
original.  Coolbeans!

Let me know if there's anything else I should do to test.  Also, let me
know if there's other pressing work to be done to this driver and I'll
try my best at de-crustifying it.

Thanks,
Chris Fester

> 
> ---
>  fs/romfs/inode.c |   37 +++++++++++++++++++++++--------------
>  1 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/romfs/inode.c b/fs/romfs/inode.c
> index 8e51a2a..60d2f82 100644
> --- a/fs/romfs/inode.c
> +++ b/fs/romfs/inode.c
> @@ -418,7 +418,8 @@ static int
>  romfs_readpage(struct file *file, struct page * page)
>  {
>  	struct inode *inode = page->mapping->host;
> -	loff_t offset, avail, readlen;
> +	loff_t offset, size;
> +	unsigned long filled;
>  	void *buf;
>  	int result = -EIO;
>  
> @@ -430,21 +431,29 @@ romfs_readpage(struct file *file, struct page * page)
>  
>  	/* 32 bit warning -- but not for us :) */
>  	offset = page_offset(page);
> -	if (offset < i_size_read(inode)) {
> -		avail = inode->i_size-offset;
> -		readlen = min_t(unsigned long, avail, PAGE_SIZE);
> -		if (romfs_copyfrom(inode, buf, ROMFS_I(inode)->i_dataoffset+offset, readlen) == readlen) {
> -			if (readlen < PAGE_SIZE) {
> -				memset(buf + readlen,0,PAGE_SIZE-readlen);
> -			}
> -			SetPageUptodate(page);
> -			result = 0;
> +	size = i_size_read(inode);
> +	filled = 0;
> +	result = 0;
> +	if (offset < size) {
> +		unsigned long readlen;
> +
> +		size -= offset;
> +		readlen = size > PAGE_SIZE ? PAGE_SIZE : size;
> +
> +		filled = romfs_copyfrom(inode, buf, ROMFS_I(inode)->i_dataoffset+offset, readlen);
> +
> +		if (filled != readlen) {
> +			SetPageError(page);
> +			filled = 0;
> +			result = -EIO;
>  		}
>  	}
> -	if (result) {
> -		memset(buf, 0, PAGE_SIZE);
> -		SetPageError(page);
> -	}
> +
> +	if (filled < PAGE_SIZE)
> +		memset(buf + filled, 0, PAGE_SIZE-filled);
> +
> +	if (!result)
> +		SetPageUptodate(page);
>  	flush_dcache_page(page);
>  
>  	unlock_page(page);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/