| Message ID | 20210604100741.18966-6-eesposit@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | blkdebug: fix racing condition when iterating on | expand |
04.06.2021 13:07, Emanuele Giuseppe Esposito wrote: > There seems to be no benefit in using a field. Replace it with a local > variable. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > --- > block/blkdebug.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/block/blkdebug.c b/block/blkdebug.c > index dffd869b32..d597753139 100644 > --- a/block/blkdebug.c > +++ b/block/blkdebug.c > @@ -40,7 +40,6 @@ > > typedef struct BDRVBlkdebugState { > int state; > - int new_state; > uint64_t align; > uint64_t max_transfer; > uint64_t opt_write_zero; > @@ -792,7 +791,7 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule) > } > > static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, > - int *action_count) > + int *action_count, int *new_state) > { > BDRVBlkdebugState *s = bs->opaque; > > @@ -812,7 +811,8 @@ static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, > break; > > case ACTION_SET_STATE: > - s->new_state = rule->options.set_state.new_state; > + assert(new_state != NULL); you don't need additional assertion: crash on dereferencing NULL pointer is not less clear anyway: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > + *new_state = rule->options.set_state.new_state; > break; > > case ACTION_SUSPEND: > @@ -825,13 +825,14 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event) > { > BDRVBlkdebugState *s = bs->opaque; > struct BlkdebugRule *rule, *next; > + int new_state; > int actions_count[ACTION__MAX] = { 0 }; > > assert((int)event >= 0 && event < BLKDBG__MAX); > > - s->new_state = s->state; > + new_state = s->state; > QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) { > - process_rule(bs, rule, actions_count); > + process_rule(bs, rule, actions_count, &new_state); > } > > while (actions_count[ACTION_SUSPEND] > 0) { > @@ -839,7 +840,7 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event) > actions_count[ACTION_SUSPEND]--; > } > > - s->state = s->new_state; > + s->state = new_state; > } > > static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event, >
diff --git a/block/blkdebug.c b/block/blkdebug.c index dffd869b32..d597753139 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -40,7 +40,6 @@ typedef struct BDRVBlkdebugState { int state; - int new_state; uint64_t align; uint64_t max_transfer; uint64_t opt_write_zero; @@ -792,7 +791,7 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule) } static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, - int *action_count) + int *action_count, int *new_state) { BDRVBlkdebugState *s = bs->opaque; @@ -812,7 +811,8 @@ static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, break; case ACTION_SET_STATE: - s->new_state = rule->options.set_state.new_state; + assert(new_state != NULL); + *new_state = rule->options.set_state.new_state; break; case ACTION_SUSPEND: @@ -825,13 +825,14 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event) { BDRVBlkdebugState *s = bs->opaque; struct BlkdebugRule *rule, *next; + int new_state; int actions_count[ACTION__MAX] = { 0 }; assert((int)event >= 0 && event < BLKDBG__MAX); - s->new_state = s->state; + new_state = s->state; QLIST_FOREACH_SAFE(rule, &s->rules[event], next, next) { - process_rule(bs, rule, actions_count); + process_rule(bs, rule, actions_count, &new_state); } while (actions_count[ACTION_SUSPEND] > 0) { @@ -839,7 +840,7 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event) actions_count[ACTION_SUSPEND]--; } - s->state = s->new_state; + s->state = new_state; } static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
There seems to be no benefit in using a field. Replace it with a local variable. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- block/blkdebug.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)