Re: [Qemu-devel] [PATCH 1/2] Block: Cleanup vvfat.c to remove dead code.

[Alex Bennée]

Prerna Saxena <address@hidden> writes:

> Commit 43dc2a64 replaced assert() with abort(), but didnt remove statements
> that followed these calls. So current code still has return values set after
> a call to abort(). Such statements will never execute and need to be cleaned
> up.
>
> Signed-off-by: Prerna Saxena <address@hidden>
> ---
>  block/vvfat.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 6b85314..ffe739b 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1747,8 +1747,7 @@ static uint32_t 
> get_cluster_count_for_direntry(BDRVVVFATState* s,
>           schedule_new_file(s, g_strdup(path), cluster_num);
>       else {
>              abort();
> -         return 0;
> -     }
> +         }
>      }
>
>      while(1) {
> @@ -1768,7 +1767,6 @@ static uint32_t 
> get_cluster_count_for_direntry(BDRVVVFATState* s,
>                           * (cluster_num - mapping->begin)) {
>                       /* offset of this cluster in file chain has changed */
>                          abort();
> -                     copy_it = 1;
>                   } else if (offset == 0) {
>                       const char* basename = get_basename(mapping->path);
>
> @@ -1780,7 +1778,6 @@ static uint32_t 
> get_cluster_count_for_direntry(BDRVVVFATState* s,
>                   if (mapping->first_mapping_index != first_mapping_index
>                           && mapping->info.file.offset > 0) {
>                          abort();
> -                     copy_it = 1;
>                   }
>
>                   /* need to write out? */
> @@ -1946,8 +1943,6 @@ DLOG(fprintf(stderr, "check direntry %d:\n", i); 
> print_direntry(direntries + i))
>               }
>           } else
>                  abort(); /* cluster_count = 0; */
> -
> -         ret += cluster_count;

I don't think you meant to do this. The ret += is not part of the else leg.

>       }
>
>       cluster_num = modified_fat_get(s, cluster_num);
> @@ -2578,10 +2573,6 @@ static int handle_commits(BDRVVVFATState* s)
>      for (i = 0; !fail && i < s->commits.next; i++) {
>       commit_t* commit = array_get(&(s->commits), i);
>       switch(commit->action) {
> -     case ACTION_RENAME: case ACTION_MKDIR:
> -            abort();
> -         fail = -2;
> -         break;
>       case ACTION_WRITEOUT: {
>  #ifndef NDEBUG
>              /* these variables are only used by assert() below */
> @@ -2639,6 +2630,8 @@ static int handle_commits(BDRVVVFATState* s)
>
>           break;
>       }
> +    case ACTION_RENAME:
> +    case ACTION_MKDIR:
>       default:
>              abort();
>       }
> @@ -2729,7 +2722,6 @@ static int do_commit(BDRVVVFATState* s)
>      if (ret) {
>       fprintf(stderr, "Error handling renames (%d)\n", ret);
>          abort();
> -     return ret;
>      }
>
>      /* copy FAT (with bdrv_read) */
> @@ -2740,21 +2732,18 @@ static int do_commit(BDRVVVFATState* s)
>      if (ret) {
>       fprintf(stderr, "Fatal: error while committing (%d)\n", ret);
>          abort();
> -     return ret;
>      }
>
>      ret = handle_commits(s);
>      if (ret) {
>       fprintf(stderr, "Error handling commits (%d)\n", ret);
>          abort();
> -     return ret;
>      }
>
>      ret = handle_deletes(s);
>      if (ret) {
>       fprintf(stderr, "Error deleting\n");
>          abort();
> -     return ret;
>      }
>
>      if (s->qcow->drv->bdrv_make_empty) {


--
Alex Bennée