Re: [PATCH 2/2] add some safety checks for label store interface of flash dev in sysfs
Seems something wrong when size = SB_LABEL_SIZE and u->label[size - 1] != '\n' please check again. --- Tang Junhui Forever me is me --- Dongbo Cao 于2018年9月14日周五 下午6:13写道: > > do some checks on the label's length and ending. > > Signed-off-by: Dongbo Cao > --- > drivers/md/bcache/sysfs.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c > index e64c718f..cce793ef 100644 > --- a/drivers/md/bcache/sysfs.c > +++ b/drivers/md/bcache/sysfs.c > @@ -447,8 +447,15 @@ STORE(__bch_flash_dev) > } > > if (attr == &sysfs_label) { > + if (size > SB_LABEL_SIZE) > + return -EINVAL; > memcpy(u->label, buf, SB_LABEL_SIZE); > - bch_uuid_write(d->c); > + if (size < SB_LABEL_SIZE) > + u->label[size] = '\0'; > + if (size && u->label[size - 1] == '\n') > + u->label[size - 1] = '\0'; > + if(d->c) > + bch_uuid_write(d->c); > } > > if (attr == &sysfs_unregister) { > -- > 2.17.1 > >
Re: [PATCH] bcache: remove unnecessary null poiner check in cache_set_free
This patch is NOT OK to me,maybe many cache in a cache_set, and some cache are not registered yet, so ca is possible NULL. zhong jiang 于2018年9月8日周六 下午7:52写道: > > The iterator in for_each_cache is never null, therefore, remove > the redundant null pointer check. > > Signed-off-by: zhong jiang > --- > drivers/md/bcache/super.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index 94c756c..2d26f9e 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1513,12 +1513,11 @@ static void cache_set_free(struct closure *cl) > bch_btree_cache_free(c); > bch_journal_free(c); > > - for_each_cache(ca, c, i) > - if (ca) { > - ca->set = NULL; > - c->cache[ca->sb.nr_this_dev] = NULL; > - kobject_put(&ca->kobj); > - } > + for_each_cache(ca, c, i) { > + ca->set = NULL; > + c->cache[ca->sb.nr_this_dev] = NULL; > + kobject_put(&ca->kobj); > + } > > bch_bset_sort_state_free(&c->sort); > free_pages((unsigned long) c->uuids, ilog2(bucket_pages(c))); > -- > 1.7.12.4 >
Re: [PATCH] bcache: remove redundant condition before debugfs_remove
LGTM Reviewed-by: tang.junhui.li...@gmail.com zhong jiang 于2018年9月8日周六 下午9:08写道: > > debugfs_remove has taken the IS_ERR_OR_NULL into account. Just > remove the unnecessary condition. > > Signed-off-by: zhong jiang > --- > drivers/md/bcache/super.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index 2d26f9e..a3d2a94 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1506,8 +1506,7 @@ static void cache_set_free(struct closure *cl) > struct cache *ca; > unsigned int i; > > - if (!IS_ERR_OR_NULL(c->debug)) > - debugfs_remove(c->debug); > + debugfs_remove(c->debug); > > bch_open_buckets_free(c); > bch_btree_cache_free(c); > -- > 1.7.12.4 >
Re: [PATCH] bcache: remove unecessary condition check before debugfs_remove_recursive
LGTM Reviewed-by: tang.junhui.li...@gmail.com zhong jiang 于2018年9月8日周六 下午9:41写道: > > debugfs_remove_recursive has taken IS_ERR_OR_NULL into account. So just > remove the condition check before debugfs_remove_recursive. > > Signed-off-by: zhong jiang > --- > drivers/md/bcache/debug.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c > index 06da66b..8c53d87 100644 > --- a/drivers/md/bcache/debug.c > +++ b/drivers/md/bcache/debug.c > @@ -249,8 +249,7 @@ void bch_debug_init_cache_set(struct cache_set *c) > > void bch_debug_exit(void) > { > - if (!IS_ERR_OR_NULL(bcache_debug)) > - debugfs_remove_recursive(bcache_debug); > + debugfs_remove_recursive(bcache_debug); > } > > void __init bch_debug_init(struct kobject *kobj) > -- > 1.7.12.4 >