Messages in this thread | | | Subject | RE: [PATCH] Propagate CRAMFS uncompression errors | Date | Mon, 9 Feb 2009 22:28:15 -0500 | From | "David VomLehn (dvomlehn)" <> |
| |
> From: Andrew Morton [mailto:akpm@linux-foundation.org] > Sent: Monday, February 09, 2009 3:17 PM > Subject: Re: [PATCH] Propagate CRAMFS uncompression errors > > "David VomLehn (dvomlehn)" <dvomlehn@cisco.com> wrote: > > > If cramfs_uncompress_block detects an error uncompressing it will > > return a zero value. This patch checks the return value and > propagates > > the error back up to the block layer. > > > > Signed-off-by: David VomLehn <dvomlehn@cisco.com> > > --- > > fs/cramfs/inode.c | 10 +++++++++- > > 1 files changed, 9 insertions(+), 1 deletions(-) > > > > diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c > > index a07338d..6ff8a5e 100644 > > --- a/fs/cramfs/inode.c > > +++ b/fs/cramfs/inode.c > > @@ -493,7 +493,15 @@ static int cramfs_readpage(struct file *file, > > struct page * page) > > Your email client is wordwrapping the patches.
Sorry. I sent it to myself first, and didn't have a problem. I hate email programs.
> > memset(pgdata + bytes_filled, 0, PAGE_CACHE_SIZE - > > bytes_filled); > > kunmap(page); > > flush_dcache_page(page); > > - SetPageUptodate(page); > > + > > + if (bytes_filled == 0) { > > + ClearPageUptodate(page); > > + SetPageError(page); > > + } > > + > > + else > > + SetPageUptodate(page); > > + > > unlock_page(page); > > return 0; > > A more typical code layout would be > > if (bytes_filled == 0) { > ClearPageUptodate(page); > SetPageError(page); > } else > SetPageUptodate(page); > > or (better, IMO): > > if (bytes_filled == 0) { > ClearPageUptodate(page); > SetPageError(page); > } else { > SetPageUptodate(page); > } > > > This patch will incorrectly cause the driver to report an IO error if > the (page->index < maxblock) test returns false. For example, a > pread() which is wholly outside the end-of-file should return > zero, not > -EIO. > > cramfs_readpage() handles this case very strangely, although not > obviously buggily. Probably this function never even gets > called for a > read wholly outside i_size. > > How does this version look to you? > > --- a/fs/cramfs/inode.c~propagate-cramfs-uncompression-errors > +++ a/fs/cramfs/inode.c > @@ -488,12 +488,19 @@ static int cramfs_readpage(struct file * > compr_len); > mutex_unlock(&read_mutex); > } > - } else > - pgdata = kmap(page); > - memset(pgdata + bytes_filled, 0, PAGE_CACHE_SIZE - > bytes_filled); > - kunmap(page); > - flush_dcache_page(page); > - SetPageUptodate(page); > + > + if (bytes_filled == 0) { > + /* Decompression error */ > + ClearPageUptodate(page); > + SetPageError(page); > + } else { > + memset(pgdata + bytes_filled, 0, > + PAGE_CACHE_SIZE - bytes_filled); > + flush_dcache_page(page); > + SetPageUptodate(page); > + } > + kunmap(page); > + } > unlock_page(page); > return 0; > }
Better, thanks!
| |