Re: [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit()

[John Snow]

On 3/29/19 4:04 PM, Zhang Chen wrote:
> From: Zhang Chen <address@hidden>
> 
> In current codes we use change_bit() to finish the job.
> 
> Signed-off-by: Zhang Chen <address@hidden>
> ---
>  include/qemu/bitmap.h |  1 -
>  include/qemu/bitops.h | 15 ---------------
>  2 files changed, 16 deletions(-)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 5c313346b9..6b71ef631c 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -52,7 +52,6 @@
>   * test_bit(bit, addr)                       Is bit set in *addr?
>   * test_and_set_bit(bit, addr)               Set bit and return old value
>   * test_and_clear_bit(bit, addr)     Clear bit and return old value
> - * test_and_change_bit(bit, addr)    Change bit and return old value
>   * find_first_zero_bit(addr, nbits)  Position first zero bit in *addr
>   * find_first_bit(addr, nbits)               Position first set bit in *addr
>   * find_next_zero_bit(addr, nbits, bit)      Position next zero bit in *addr 
> >= bit
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 3f0926cf40..1f98ffcdc0 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -109,21 +109,6 @@ static inline int test_and_clear_bit(long nr, unsigned 
> long *addr)
>      return (old & mask) != 0;
>  }
>  
> -/**
> - * test_and_change_bit - Change a bit and return its old value
> - * @nr: Bit to change
> - * @addr: Address to count from
> - */
> -static inline int test_and_change_bit(long nr, unsigned long *addr)
> -{
> -    unsigned long mask = BIT_MASK(nr);
> -    unsigned long *p = addr + BIT_WORD(nr);
> -    unsigned long old = *p;
> -
> -    *p = old ^ mask;
> -    return (old & mask) != 0;
> -}
> -
>  /**
>   * test_bit - Determine whether a bit is set
>   * @nr: bit number to test
> 

I personally don't see the harm in keeping this, but it is indeed
unused, so:

Reviewed-by: John Snow <address@hidden>


As for merging other sibling functions, I guess the desire is a small
decrease in SLOC; I'm not sure if you'll run into any uses where the
changed signatures for a combined function causes issues. I am not sure
it's worth the hassle.