lkml.org 
[lkml]   [2009]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRE: [PATCH] Propagate CRAMFS uncompression errors
Date
From

> 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!


\
 
 \ /
  Last update: 2009-02-10 04:41    [W:0.066 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site