Re: [PATCH v2 3/3] lib: rename lib/find_next_bit.c to lib/find_bit.c

[Rasmus Villemoes]

On Sat, Jan 31 2015, yury.norov@xxxxxxxxx wrote:

> From: Yury Norov <yury.norov@xxxxxxxxx>
>
> This file contains implementation for:
> - find_last_bit;
> - find_first_zero_bit;
> - find_first_bit;
> - find_next_zero_bit;
> - find_next_bit.
>

[and a few _le variants]

> So giving more generic name looks reasonable.

It does. But it is a little annoying that it is not shown as a pure
rename.

> Signed-off-by: Yury Norov <yury.norov@xxxxxxxxx>
> ---
>  lib/Makefile        |   2 +-
>  lib/find_bit.c      | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/find_next_bit.c | 192 ---------------------------------------------------
>  3 files changed, 195 insertions(+), 193 deletions(-)
>  create mode 100644 lib/find_bit.c
>  delete mode 100644 lib/find_next_bit.c
>
> diff --git a/lib/Makefile b/lib/Makefile
> index 13990aa..1cc93f4 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -25,7 +25,7 @@ obj-y	+= lockref.o
>  obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
>  	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
>  	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
> -	 bsearch.o find_next_bit.o llist.o memweight.o kfifo.o \
> +	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
>  	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
>  obj-y += string_helpers.o
>  obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
> diff --git a/lib/find_bit.c b/lib/find_bit.c
> new file mode 100644
> index 0000000..236a850
> --- /dev/null
> +++ b/lib/find_bit.c
> @@ -0,0 +1,194 @@
> +/* find_bit.c: generic implementation fore:
> + * find_last_bit; find_first_zero_bit; find_first_bit;
> + * find_next_zero_bit; find_next_bit.
> + *

[trivial typo: s/fore/for/]

I _think_ the cause of the 2-line descrepancy is this change in a comment,
but it's hard to tell for sure. There's no simple way of telling whether the
other 192 lines are actually the same or if subtle changes have been
introduced.

This is one reason hard-coding the name of a file inside the file is a
bad idea. I'd suggest tweaking that comment in one of the earlier
patches, for example the one moving find_last_bit to find_next_bit.c,
making sure to remove the filename. Then this patch can be a simple 'git
mv' and the same two-line change of the Makefile.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/