Hi, On Oct 30, 2012, at 5:23 PM, Alan Cox wrote: > From: Alan Cox <alan@xxxxxxxxxxxxxxx> > > If the read fails we kmap an error code. This doesn't end well. Instead > print a critical error and pray. This mirrors the rest of the fs behaviour > with critical error cases. > > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx> > --- > > fs/hfsplus/bitmap.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > > diff --git a/fs/hfsplus/bitmap.c b/fs/hfsplus/bitmap.c > index 4cfbe2e..5eeefee 100644 > --- a/fs/hfsplus/bitmap.c > +++ b/fs/hfsplus/bitmap.c > @@ -182,6 +182,8 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count) > mapping = sbi->alloc_file->i_mapping; > pnr = offset / PAGE_CACHE_BITS; > page = read_mapping_page(mapping, pnr, NULL); > + if (IS_ERR(page)) > + goto kaboom; > pptr = kmap(page); > curr = pptr + (offset & (PAGE_CACHE_BITS - 1)) / 32; > end = pptr + PAGE_CACHE_BITS / 32; > @@ -214,6 +216,8 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count) > set_page_dirty(page); > kunmap(page); > page = read_mapping_page(mapping, ++pnr, NULL); > + if (IS_ERR(page)) > + goto kaboom; > pptr = kmap(page); > curr = pptr; > end = pptr + PAGE_CACHE_BITS / 32; > @@ -231,5 +235,9 @@ out: > hfsplus_mark_mdb_dirty(sb); > mutex_unlock(&sbi->alloc_mutex); > > +kaboom: > + printk(KERN_CRIT "hfsplus: unable to mark blocks free: error %ld\n", > + PTR_ERR(page)); > + mutex_unlock(&sbi->alloc_mutex); Your patch is good. But what about returning an error code (for example, -EIO)? Unfortunately, as I can see, the code that call hfsplus_block_free() doesn't analyze return code. I think that it is not good. With the best regards, Vyacheslav Dubeyko. > return 0; > } > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
