Message ID | 20230915184130.403366-8-den@openvz.org |
---|---|
State | New |
Headers | show |
Series | implement discard operation for Parallels images | expand |
This patch generates a warning. On Fri, Sep 15, 2023 at 9:41 PM Denis V. Lunev <den@openvz.org> wrote: > > More conditions follows thus the check should be more scalable. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > --- > block/parallels.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 8f223bfd89..aa29df9f77 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -1065,7 +1065,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > int ret, size, i; > int64_t file_nb_sectors, sector; > uint32_t data_start; > - bool data_off_is_correct; > + bool need_check = false; > > ret = parallels_opts_prealloc(bs, options, errp); > if (ret < 0) { > @@ -1133,11 +1133,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > s->bat_bitmap = (uint32_t *)(s->header + 1); > > if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { > - s->header_unclean = true; > + need_check = s->header_unclean = true; > } > > - data_off_is_correct = parallels_test_data_off(s, file_nb_sectors, > - &data_start); > + need_check = need_check || > + !parallels_test_data_off(s, file_nb_sectors, &data_start); > + ../block/parallels.c:1139:18: warning: variable 'data_start' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized] need_check = need_check || ^~~~~~~~~~ ../block/parallels.c:1142:21: note: uninitialized use occurs here s->data_start = data_start; ^~~~~~~~~~ ../block/parallels.c:1139:18: note: remove the '||' if its condition is always false need_check = need_check || ^~~~~~~~~~~~~ ../block/parallels.c:1067:24: note: initialize the variable 'data_start' to silence this warning uint32_t data_start; ^ = 0 1 warning generated. Regards, Mike.
On 9/17/23 13:40, Mike Maslenkin wrote: > This patch generates a warning. > > On Fri, Sep 15, 2023 at 9:41 PM Denis V. Lunev <den@openvz.org> wrote: >> More conditions follows thus the check should be more scalable. >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> --- >> block/parallels.c | 19 ++++++++----------- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 8f223bfd89..aa29df9f77 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -1065,7 +1065,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> int ret, size, i; >> int64_t file_nb_sectors, sector; >> uint32_t data_start; >> - bool data_off_is_correct; >> + bool need_check = false; >> >> ret = parallels_opts_prealloc(bs, options, errp); >> if (ret < 0) { >> @@ -1133,11 +1133,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> s->bat_bitmap = (uint32_t *)(s->header + 1); >> >> if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { >> - s->header_unclean = true; >> + need_check = s->header_unclean = true; >> } >> >> - data_off_is_correct = parallels_test_data_off(s, file_nb_sectors, >> - &data_start); >> + need_check = need_check || >> + !parallels_test_data_off(s, file_nb_sectors, &data_start); >> + > ../block/parallels.c:1139:18: warning: variable 'data_start' is used > uninitialized whenever '||' condition is true > [-Wsometimes-uninitialized] > need_check = need_check || > ^~~~~~~~~~ > ../block/parallels.c:1142:21: note: uninitialized use occurs here > s->data_start = data_start; > ^~~~~~~~~~ > ../block/parallels.c:1139:18: note: remove the '||' if its condition > is always false > need_check = need_check || > ^~~~~~~~~~~~~ > ../block/parallels.c:1067:24: note: initialize the variable > 'data_start' to silence this warning > uint32_t data_start; > ^ > = 0 > 1 warning generated. > > > Regards, > Mike. wow! That is pretty much correct! What compiler/OS you are using :) Den
On 9/15/23 20:41, Denis V. Lunev wrote: > More conditions follows thus the check should be more scalable. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > --- > block/parallels.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 8f223bfd89..aa29df9f77 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -1065,7 +1065,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > int ret, size, i; > int64_t file_nb_sectors, sector; > uint32_t data_start; > - bool data_off_is_correct; > + bool need_check = false; > > ret = parallels_opts_prealloc(bs, options, errp); > if (ret < 0) { > @@ -1133,11 +1133,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > s->bat_bitmap = (uint32_t *)(s->header + 1); > > if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { > - s->header_unclean = true; > + need_check = s->header_unclean = true; > } > > - data_off_is_correct = parallels_test_data_off(s, file_nb_sectors, > - &data_start); > + need_check = need_check || > + !parallels_test_data_off(s, file_nb_sectors, &data_start); > + > s->data_start = data_start; > s->data_end = s->data_start; > if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) { > @@ -1194,6 +1195,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > s->data_end = sector + s->tracks; > } > } > + need_check = need_check || s->data_end > file_nb_sectors; > > /* > * We don't repair the image here if it's opened for checks. Also we don't > @@ -1203,12 +1205,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > return 0; > } > > - /* > - * Repair the image if it's dirty or > - * out-of-image corruption was detected. > - */ > - if (s->data_end > file_nb_sectors || s->header_unclean > - || !data_off_is_correct) { > + /* Repair the image if corruption was detected. */ > + if (need_check) { > BdrvCheckResult res; > ret = bdrv_check(bs, &res, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS); > if (ret < 0) { > @@ -1217,7 +1215,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > } > - > return 0; > > fail_format: Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
diff --git a/block/parallels.c b/block/parallels.c index 8f223bfd89..aa29df9f77 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -1065,7 +1065,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, int ret, size, i; int64_t file_nb_sectors, sector; uint32_t data_start; - bool data_off_is_correct; + bool need_check = false; ret = parallels_opts_prealloc(bs, options, errp); if (ret < 0) { @@ -1133,11 +1133,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, s->bat_bitmap = (uint32_t *)(s->header + 1); if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { - s->header_unclean = true; + need_check = s->header_unclean = true; } - data_off_is_correct = parallels_test_data_off(s, file_nb_sectors, - &data_start); + need_check = need_check || + !parallels_test_data_off(s, file_nb_sectors, &data_start); + s->data_start = data_start; s->data_end = s->data_start; if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) { @@ -1194,6 +1195,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, s->data_end = sector + s->tracks; } } + need_check = need_check || s->data_end > file_nb_sectors; /* * We don't repair the image here if it's opened for checks. Also we don't @@ -1203,12 +1205,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, return 0; } - /* - * Repair the image if it's dirty or - * out-of-image corruption was detected. - */ - if (s->data_end > file_nb_sectors || s->header_unclean - || !data_off_is_correct) { + /* Repair the image if corruption was detected. */ + if (need_check) { BdrvCheckResult res; ret = bdrv_check(bs, &res, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS); if (ret < 0) { @@ -1217,7 +1215,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } } - return 0; fail_format:
More conditions follows thus the check should be more scalable. Signed-off-by: Denis V. Lunev <den@openvz.org> --- block/parallels.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)