From b70ea39d4f2547564197afa8866b388301765583 Mon Sep 17 00:00:00 2001 From: Charles Manning Date: Mon, 10 Jan 2011 11:58:11 +1300 Subject: [PATCH 1/1] yaffs: Sort out issues raised by Coverity Coverity checks raised some issues, particularly with NULL checks. Fix them. Signed-off-by: Charles Manning --- yaffs_guts.c | 61 +++++++++++++++++++++------------------------- yaffs_nand.c | 1 + yaffs_tagscompat.c | 4 +-- yaffs_verify.c | 37 ++++++++++++---------------- yaffs_vfs_multi.c | 33 +++++++++++++------------ yaffs_vfs_single.c | 34 ++++++++++++++------------ yaffs_yaffs2.c | 35 ++++++++++++-------------- 7 files changed, 98 insertions(+), 107 deletions(-) diff --git a/yaffs_guts.c b/yaffs_guts.c index f4ae9de..486b7e0 100644 --- a/yaffs_guts.c +++ b/yaffs_guts.c @@ -1532,7 +1532,7 @@ static struct yaffs_cache *yaffs_grab_chunk_cache(struct yaffs_dev *dev) /* With locking we can't assume we can use entry zero */ - the_obj = NULL; + the_obj = dev->cache[0].object; usage = -1; cache = NULL; pushout = -1; @@ -1952,16 +1952,12 @@ struct yaffs_obj *yaffs_find_by_number(struct yaffs_dev *dev, u32 number) list_for_each(i, &dev->obj_bucket[bucket].list) { /* Look if it is in the list */ - if (i) { - in = list_entry(i, struct yaffs_obj, hash_link); - if (in->obj_id == number) { - - /* Don't tell the VFS about this one if it is defered free */ - if (in->defered_free) - return NULL; - - return in; - } + in = list_entry(i, struct yaffs_obj, hash_link); + if (in->obj_id == number) { + /* Don't tell the VFS about this one if it is defered free */ + if (in->defered_free) + return NULL; + return in; } } @@ -4082,11 +4078,13 @@ static int yaffs_unlink_worker(struct yaffs_obj *obj) int del_now = 0; + if(!obj) + return YAFFS_FAIL; + if (!obj->my_inode) del_now = 1; - if (obj) - yaffs_update_parent(obj->parent); + yaffs_update_parent(obj->parent); if (obj->variant_type == YAFFS_OBJECT_TYPE_HARDLINK) { return yaffs_del_link(obj); @@ -4496,29 +4494,26 @@ struct yaffs_obj *yaffs_find_by_name(struct yaffs_obj *directory, sum = yaffs_calc_name_sum(name); list_for_each(i, &directory->variant.dir_variant.children) { - if (i) { - l = list_entry(i, struct yaffs_obj, siblings); + l = list_entry(i, struct yaffs_obj, siblings); - if (l->parent != directory) - YBUG(); + if (l->parent != directory) + YBUG(); - yaffs_check_obj_details_loaded(l); + yaffs_check_obj_details_loaded(l); - /* Special case for lost-n-found */ - if (l->obj_id == YAFFS_OBJECTID_LOSTNFOUND) { - if (!strcmp(name, YAFFS_LOSTNFOUND_NAME)) - return l; - } else if (l->sum == sum - || l->hdr_chunk <= 0) { - /* LostnFound chunk called Objxxx - * Do a real check - */ - yaffs_get_obj_name(l, buffer, - YAFFS_MAX_NAME_LENGTH + 1); - if (strncmp - (name, buffer, YAFFS_MAX_NAME_LENGTH) == 0) - return l; - } + /* Special case for lost-n-found */ + if (l->obj_id == YAFFS_OBJECTID_LOSTNFOUND) { + if (!strcmp(name, YAFFS_LOSTNFOUND_NAME)) + return l; + } else if (l->sum == sum + || l->hdr_chunk <= 0) { + /* LostnFound chunk called Objxxx + * Do a real check + */ + yaffs_get_obj_name(l, buffer, + YAFFS_MAX_NAME_LENGTH + 1); + if (strncmp(name, buffer, YAFFS_MAX_NAME_LENGTH) == 0) + return l; } } diff --git a/yaffs_nand.c b/yaffs_nand.c index e816cab..f7810d0 100644 --- a/yaffs_nand.c +++ b/yaffs_nand.c @@ -72,6 +72,7 @@ int yaffs_wr_chunk_tags_nand(struct yaffs_dev *dev, } else { yaffs_trace(YAFFS_TRACE_ERROR, "Writing with no tags"); YBUG(); + return FALSE; } if (dev->param.write_chunk_tags_fn) diff --git a/yaffs_tagscompat.c b/yaffs_tagscompat.c index 7578075..818f72b 100644 --- a/yaffs_tagscompat.c +++ b/yaffs_tagscompat.c @@ -151,7 +151,7 @@ static int yaffs_rd_chunk_nand(struct yaffs_dev *dev, int ret_val; struct yaffs_spare local_spare; - if (!spare && data) { + if (!spare) { /* If we don't have a real spare, then we use a local one. */ /* Need this for the calculation of the ecc */ spare = &local_spare; @@ -290,7 +290,7 @@ int yaffs_tags_compat_wr(struct yaffs_dev *dev, struct yaffs_tags tags; yaffs_spare_init(&spare); - + if (ext_tags->is_deleted) spare.page_status = 0; else { diff --git a/yaffs_verify.c b/yaffs_verify.c index 738c7f6..55884b9 100644 --- a/yaffs_verify.c +++ b/yaffs_verify.c @@ -163,8 +163,8 @@ void yaffs_verify_blocks(struct yaffs_dev *dev) } /* - * Verify the object header. oh must be valid, but obj and tags may be NULL in which - * case those tests will not be performed. + * Verify the object header. oh must be valid, but obj and tags may be NULL in + * which case those tests will not be performed. */ void yaffs_verify_oh(struct yaffs_obj *obj, struct yaffs_obj_hdr *oh, struct yaffs_ext_tags *tags, int parent_check) @@ -414,11 +414,8 @@ void yaffs_verify_objects(struct yaffs_dev *dev) for (i = 0; i < YAFFS_NOBJECT_BUCKETS; i++) { list_for_each(lh, &dev->obj_bucket[i].list) { - if (lh) { - obj = - list_entry(lh, struct yaffs_obj, hash_link); - yaffs_verify_obj(obj); - } + obj = list_entry(lh, struct yaffs_obj, hash_link); + yaffs_verify_obj(obj); } } } @@ -453,12 +450,10 @@ void yaffs_verify_obj_in_dir(struct yaffs_obj *obj) /* Iterate through the objects in each hash entry */ list_for_each(lh, &obj->parent->variant.dir_variant.children) { - if (lh) { - list_obj = list_entry(lh, struct yaffs_obj, siblings); - yaffs_verify_obj(list_obj); - if (obj == list_obj) - count++; - } + list_obj = list_entry(lh, struct yaffs_obj, siblings); + yaffs_verify_obj(list_obj); + if (obj == list_obj) + count++; } if (count != 1) { @@ -492,16 +487,14 @@ void yaffs_verify_dir(struct yaffs_obj *directory) /* Iterate through the objects in each hash entry */ list_for_each(lh, &directory->variant.dir_variant.children) { - if (lh) { - list_obj = list_entry(lh, struct yaffs_obj, siblings); - if (list_obj->parent != directory) { - yaffs_trace(YAFFS_TRACE_ALWAYS, - "Object in directory list has wrong parent %p", - list_obj->parent); - YBUG(); - } - yaffs_verify_obj_in_dir(list_obj); + list_obj = list_entry(lh, struct yaffs_obj, siblings); + if (list_obj->parent != directory) { + yaffs_trace(YAFFS_TRACE_ALWAYS, + "Object in directory list has wrong parent %p", + list_obj->parent); + YBUG(); } + yaffs_verify_obj_in_dir(list_obj); } } diff --git a/yaffs_vfs_multi.c b/yaffs_vfs_multi.c index 9feecdf..78e2f9f 100644 --- a/yaffs_vfs_multi.c +++ b/yaffs_vfs_multi.c @@ -692,11 +692,9 @@ static void yaffs_remove_obj_callback(struct yaffs_obj *obj) * the search context to the next object to prevent a hanging pointer. */ list_for_each(i, search_contexts) { - if (i) { - sc = list_entry(i, struct yaffs_search_context, others); - if (sc->next_return == obj) - yaffs_search_advance(sc); - } + sc = list_entry(i, struct yaffs_search_context, others); + if (sc->next_return == obj) + yaffs_search_advance(sc); } } @@ -1421,6 +1419,12 @@ static ssize_t yaffs_file_write(struct file *f, const char *buf, size_t n, obj = yaffs_dentry_to_obj(f->f_dentry); + if (!obj) { + yaffs_trace(YAFFS_TRACE_OS, + "yaffs_file_write: hey obj is null!"); + return -EINVAL; + } + dev = obj->my_dev; yaffs_gross_lock(dev); @@ -1432,13 +1436,9 @@ static ssize_t yaffs_file_write(struct file *f, const char *buf, size_t n, else ipos = *pos; - if (!obj) - yaffs_trace(YAFFS_TRACE_OS, - "yaffs_file_write: hey obj is null!"); - else - yaffs_trace(YAFFS_TRACE_OS, - "yaffs_file_write about to write writing %u(%x) bytes to object %d at %d(%x)", - (unsigned)n, (unsigned)n, obj->obj_id, ipos, ipos); + yaffs_trace(YAFFS_TRACE_OS, + "yaffs_file_write about to write writing %u(%x) bytes to object %d at %d(%x)", + (unsigned)n, (unsigned)n, obj->obj_id, ipos, ipos); n_written = yaffs_wr_file(obj, buf, ipos, n, 0); @@ -2623,6 +2623,11 @@ static struct super_block *yaffs_internal_read_super(int yaffs_version, struct yaffs_linux_context *context_iterator; struct list_head *l; + if (!sb) { + printk(KERN_INFO "yaffs: sb is NULL\n"); + return NULL; + } + sb->s_magic = YAFFS_MAGIC; sb->s_op = &yaffs_super_ops; sb->s_flags |= MS_NOATIME; @@ -2633,9 +2638,7 @@ static struct super_block *yaffs_internal_read_super(int yaffs_version, sb->s_export_op = &yaffs_export_ops; #endif - if (!sb) - printk(KERN_INFO "yaffs: sb is NULL\n"); - else if (!sb->s_dev) + if (!sb->s_dev) printk(KERN_INFO "yaffs: sb->s_dev is NULL\n"); else if (!yaffs_devname(sb, devname_buf)) printk(KERN_INFO "yaffs: devname is NULL\n"); diff --git a/yaffs_vfs_single.c b/yaffs_vfs_single.c index 2cd6047..1893125 100644 --- a/yaffs_vfs_single.c +++ b/yaffs_vfs_single.c @@ -783,11 +783,9 @@ static void yaffs_remove_obj_callback(struct yaffs_obj *obj) * the search context to the next object to prevent a hanging pointer. */ list_for_each(i, search_contexts) { - if (i) { - sc = list_entry(i, struct yaffs_search_context, others); - if (sc->next_return == obj) - yaffs_search_advance(sc); - } + sc = list_entry(i, struct yaffs_search_context, others); + if (sc->next_return == obj) + yaffs_search_advance(sc); } } @@ -1376,6 +1374,13 @@ static ssize_t yaffs_file_write(struct file *f, const char *buf, size_t n, obj = yaffs_dentry_to_obj(f->f_dentry); + if (!obj) { + /* This should not happen */ + yaffs_trace(YAFFS_TRACE_OS, + "yaffs_file_write: hey obj is null!"); + return -ENINVAL; + } + dev = obj->my_dev; yaffs_gross_lock(dev); @@ -1387,13 +1392,9 @@ static ssize_t yaffs_file_write(struct file *f, const char *buf, size_t n, else ipos = *pos; - if (!obj) - yaffs_trace(YAFFS_TRACE_OS, - "yaffs_file_write: hey obj is null!"); - else - yaffs_trace(YAFFS_TRACE_OS, - "yaffs_file_write about to write writing %u(%x) bytes to object %d at %d(%x)", - (unsigned)n, (unsigned)n, obj->obj_id, ipos, ipos); + yaffs_trace(YAFFS_TRACE_OS, + "yaffs_file_write about to write writing %u(%x) bytes to object %d at %d(%x)", + (unsigned)n, (unsigned)n, obj->obj_id, ipos, ipos); n_written = yaffs_wr_file(obj, buf, ipos, n, 0); @@ -2027,6 +2028,11 @@ static struct super_block *yaffs_internal_read_super(int yaffs_version, struct yaffs_linux_context *context_iterator; struct list_head *l; + if (!sb) { + printk(KERN_INFO "yaffs: sb is NULL\n"); + return NULL; + } + sb->s_magic = YAFFS_MAGIC; sb->s_op = &yaffs_super_ops; sb->s_flags |= MS_NOATIME; @@ -2035,9 +2041,7 @@ static struct super_block *yaffs_internal_read_super(int yaffs_version, sb->s_export_op = &yaffs_export_ops; - if (!sb) - printk(KERN_INFO "yaffs: sb is NULL\n"); - else if (!sb->s_dev) + if (!sb->s_dev) printk(KERN_INFO "yaffs: sb->s_dev is NULL\n"); else if (!yaffs_devname(sb, devname_buf)) printk(KERN_INFO "yaffs: devname is NULL\n"); diff --git a/yaffs_yaffs2.c b/yaffs_yaffs2.c index 33397af..339f47f 100644 --- a/yaffs_yaffs2.c +++ b/yaffs_yaffs2.c @@ -568,28 +568,23 @@ static int yaffs2_wr_checkpt_objs(struct yaffs_dev *dev) for (i = 0; ok && i < YAFFS_NOBJECT_BUCKETS; i++) { list_for_each(lh, &dev->obj_bucket[i].list) { - if (lh) { - obj = - list_entry(lh, struct yaffs_obj, hash_link); - if (!obj->defered_free) { - yaffs2_obj_checkpt_obj(&cp, obj); - cp.struct_type = sizeof(cp); - - yaffs_trace(YAFFS_TRACE_CHECKPOINT, - "Checkpoint write object %d parent %d type %d chunk %d obj addr %p", - cp.obj_id, cp.parent_id, - cp.variant_type, cp.hdr_chunk, obj); - - ok = (yaffs2_checkpt_wr - (dev, &cp, + obj = list_entry(lh, struct yaffs_obj, hash_link); + if (!obj->defered_free) { + yaffs2_obj_checkpt_obj(&cp, obj); + cp.struct_type = sizeof(cp); + + yaffs_trace(YAFFS_TRACE_CHECKPOINT, + "Checkpoint write object %d parent %d type %d chunk %d obj addr %p", + cp.obj_id, cp.parent_id, + cp.variant_type, cp.hdr_chunk, obj); + + ok = (yaffs2_checkpt_wr(dev, &cp, sizeof(cp)) == sizeof(cp)); - if (ok - && obj->variant_type == - YAFFS_OBJECT_TYPE_FILE) - ok = yaffs2_wr_checkpt_tnodes - (obj); - } + if (ok && + obj->variant_type == + YAFFS_OBJECT_TYPE_FILE) + ok = yaffs2_wr_checkpt_tnodes(obj); } } } -- 2.30.2