block: Simplify bdrv_append_temp_snapshot() logic
Noticed while checking Coccinelle results. Naming a label 'out:' when it is only used on error paths is weird. Also, we had some dead stores to 'ret'. Meanwhile we know that snapshot_options is NULL on success and that QDECREF(NULL) is safe. So merge the two exit paths into one by careful control over bs_snapshot. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170427215821.19397-8-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
		
							parent
							
								
									46f5ac205a
								
							
						
					
					
						commit
						ff6ed7141d
					
				
							
								
								
									
										17
									
								
								block.c
									
									
									
									
									
								
							
							
						
						
									
										17
									
								
								block.c
									
									
									
									
									
								
							@ -2195,7 +2195,7 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
 | 
				
			|||||||
    char *tmp_filename = g_malloc0(PATH_MAX + 1);
 | 
					    char *tmp_filename = g_malloc0(PATH_MAX + 1);
 | 
				
			||||||
    int64_t total_size;
 | 
					    int64_t total_size;
 | 
				
			||||||
    QemuOpts *opts = NULL;
 | 
					    QemuOpts *opts = NULL;
 | 
				
			||||||
    BlockDriverState *bs_snapshot;
 | 
					    BlockDriverState *bs_snapshot = NULL;
 | 
				
			||||||
    Error *local_err = NULL;
 | 
					    Error *local_err = NULL;
 | 
				
			||||||
    int ret;
 | 
					    int ret;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@ -2235,28 +2235,25 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
 | 
				
			|||||||
    bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
 | 
					    bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
 | 
				
			||||||
    snapshot_options = NULL;
 | 
					    snapshot_options = NULL;
 | 
				
			||||||
    if (!bs_snapshot) {
 | 
					    if (!bs_snapshot) {
 | 
				
			||||||
        ret = -EINVAL;
 | 
					 | 
				
			||||||
        goto out;
 | 
					        goto out;
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    /* bdrv_append() consumes a strong reference to bs_snapshot (i.e. it will
 | 
					    /* bdrv_append() consumes a strong reference to bs_snapshot
 | 
				
			||||||
     * call bdrv_unref() on it), so in order to be able to return one, we have
 | 
					     * (i.e. it will call bdrv_unref() on it) even on error, so in
 | 
				
			||||||
     * to increase bs_snapshot's refcount here */
 | 
					     * order to be able to return one, we have to increase
 | 
				
			||||||
 | 
					     * bs_snapshot's refcount here */
 | 
				
			||||||
    bdrv_ref(bs_snapshot);
 | 
					    bdrv_ref(bs_snapshot);
 | 
				
			||||||
    bdrv_append(bs_snapshot, bs, &local_err);
 | 
					    bdrv_append(bs_snapshot, bs, &local_err);
 | 
				
			||||||
    if (local_err) {
 | 
					    if (local_err) {
 | 
				
			||||||
        error_propagate(errp, local_err);
 | 
					        error_propagate(errp, local_err);
 | 
				
			||||||
        ret = -EINVAL;
 | 
					        bs_snapshot = NULL;
 | 
				
			||||||
        goto out;
 | 
					        goto out;
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    g_free(tmp_filename);
 | 
					 | 
				
			||||||
    return bs_snapshot;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
out:
 | 
					out:
 | 
				
			||||||
    QDECREF(snapshot_options);
 | 
					    QDECREF(snapshot_options);
 | 
				
			||||||
    g_free(tmp_filename);
 | 
					    g_free(tmp_filename);
 | 
				
			||||||
    return NULL;
 | 
					    return bs_snapshot;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/*
 | 
					/*
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user