Re: [Qemu-block] [Qemu-devel] [PATCH 2/6] qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty()

[Stefan Hajnoczi]

On Thu, Apr 30, 2015 at 01:11:41PM +0300, Alberto Garcia wrote:
>  int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
>  {
> -    int i;
> +    int i = (*table - c->table_array) / c->table_size;
>  
> -    for (i = 0; i < c->size; i++) {
> -        if (table_addr(c, i) == *table) {
> -            goto found;
> -        }
> +    if (c->entries[i].offset == 0) {
> +        return -ENOENT;
>      }
> -    return -ENOENT;
>  
> -found:
>      c->entries[i].ref--;
>      *table = NULL;
>  

When is this function called with a bogus table pointer?

My hunch is there are no callers like that.  I might be wrong.

If there are no callers like that, then you can extract this and include
an assertion instead:

static int qcow2_cache_get_table_idx(Qcow2Cache *c, void *table)
{
    assert(table >= c->table_array &&);
           table < c->table_array + c->size * c->table_size);
    return (table - c->table_array) / c->table_size;
}

Functions that go from void *table to int i should use this helper.

And the qcow2_cache_put() users can drop their error handling code.  A
lot of them ignore the return value anyway, so it simplifies things even
more.

Stefan

pgpAbixDmmk_8.pgp
Description: PGP signature