| Message ID | 20180315150814.9412-13-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/03/2018 11:08 PM, Bart Van Assche wrote: > This patch does not change any functionality but avoids that smatch > reports the following: > > drivers/md/bcache/alloc.c:334: bch_allocator_thread() error: uninitialized symbol 'bucket'. > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Hi Bart, Your change is OK to me, but the original code seems OK too. Can I say this is a bug should be fixed from smatch ? Thanks. Coly Li > --- > drivers/md/bcache/alloc.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c > index 458e1d38577d..6a98d48c5f5f 100644 > --- a/drivers/md/bcache/alloc.c > +++ b/drivers/md/bcache/alloc.c > @@ -316,6 +316,7 @@ static int bch_allocator_push(struct cache *ca, long bucket) > static int bch_allocator_thread(void *arg) > { > struct cache *ca = arg; > + long bucket; > > mutex_lock(&ca->set->bucket_lock); > > @@ -325,11 +326,7 @@ static int bch_allocator_thread(void *arg) > * possibly issue discards to them, then we add the bucket to > * the free list: > */ > - while (!fifo_empty(&ca->free_inc)) { > - long bucket; > - > - fifo_pop(&ca->free_inc, bucket); > - > + while (fifo_pop(&ca->free_inc, bucket)) { > if (ca->discard) { > mutex_unlock(&ca->set->bucket_lock); > blkdev_issue_discard(ca->bdev, >
On Fri, 2018-03-16 at 00:29 +0800, Coly Li wrote: > On 15/03/2018 11:08 PM, Bart Van Assche wrote: > > This patch does not change any functionality but avoids that smatch > > reports the following: > > > > drivers/md/bcache/alloc.c:334: bch_allocator_thread() error: uninitialized symbol 'bucket'. > > > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> > > Hi Bart, > > Your change is OK to me, but the original code seems OK too. Can I say > this is a bug should be fixed from smatch ? Hello Coly, It would be great if smatch would be modified such that it does not complain about this code. But I think that this patch does not only make the code easier to analyze for static analyzers but also for humans. So please evaluate this patch from that perspective. Thanks, Bart.
On 16/03/2018 12:52 AM, Bart Van Assche wrote: > On Fri, 2018-03-16 at 00:29 +0800, Coly Li wrote: >> On 15/03/2018 11:08 PM, Bart Van Assche wrote: >>> This patch does not change any functionality but avoids that smatch >>> reports the following: >>> >>> drivers/md/bcache/alloc.c:334: bch_allocator_thread() error: uninitialized symbol 'bucket'. >>> >>> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> >> >> Hi Bart, >> >> Your change is OK to me, but the original code seems OK too. Can I say >> this is a bug should be fixed from smatch ? > > Hello Coly, > > It would be great if smatch would be modified such that it does not complain > about this code. But I think that this patch does not only make the code > easier to analyze for static analyzers but also for humans. So please evaluate > this patch from that perspective. Hi Bart, It's fair enough, I am convinced. Thanks. Reviewed-by: Coly Li <colyli@suse.de> Coly Li
diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c index 458e1d38577d..6a98d48c5f5f 100644 --- a/drivers/md/bcache/alloc.c +++ b/drivers/md/bcache/alloc.c @@ -316,6 +316,7 @@ static int bch_allocator_push(struct cache *ca, long bucket) static int bch_allocator_thread(void *arg) { struct cache *ca = arg; + long bucket; mutex_lock(&ca->set->bucket_lock); @@ -325,11 +326,7 @@ static int bch_allocator_thread(void *arg) * possibly issue discards to them, then we add the bucket to * the free list: */ - while (!fifo_empty(&ca->free_inc)) { - long bucket; - - fifo_pop(&ca->free_inc, bucket); - + while (fifo_pop(&ca->free_inc, bucket)) { if (ca->discard) { mutex_unlock(&ca->set->bucket_lock); blkdev_issue_discard(ca->bdev,
This patch does not change any functionality but avoids that smatch reports the following: drivers/md/bcache/alloc.c:334: bch_allocator_thread() error: uninitialized symbol 'bucket'. Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> --- drivers/md/bcache/alloc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)