Remove duplicate definition of ARRAY_SIZE(), which was never used anyway.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
kernel/range.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/kernel/range.c b/kernel/range.c
index 74e2e61..471b66a 100644
--- a/kernel/range.c
+++ b/kernel/range.c
@@ -7,10 +7,6 @@
#include <linux/range.h>
-#ifndef ARRAY_SIZE
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-#endif
-
int add_range(struct range *range, int az, int nr_range, u64 start, u64 end)
{
if (start >= end)
--
1.7.0.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sat, 5 Jun 2010 21:32:15 +0200 (CEST)
Geert Uytterhoeven <[email protected]> wrote:
> Remove duplicate definition of ARRAY_SIZE(), which was never used anyway.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> kernel/range.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/range.c b/kernel/range.c
> index 74e2e61..471b66a 100644
> --- a/kernel/range.c
> +++ b/kernel/range.c
> @@ -7,10 +7,6 @@
>
> #include <linux/range.h>
>
> -#ifndef ARRAY_SIZE
> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> -#endif
> -
> int add_range(struct range *range, int az, int nr_range, u64 start, u64 end)
> {
> if (start >= end)
<discovers range.c>
That's not terribly great code, sorry.
- The names are all wrong. Should be range_add(),
range_add_with_merge(), range_subtract(), etc.
- It's completely undocumented!
- It's linked into every vmlinux in the world, many of which won't use it
afacit.
- The return value from add_range() is a bit odd. I guess callers must do
if (add_range(..., ..., nr_range, ..., ...) == nr_range)
error()
- What does the identifier "az" mean?
- `az' and `nr_range' should be unsigned types. That would make the
"Out of slots:" check non-buggy.
- The return value from add_range_with_merge() is unusable! If it
did a merge into the final range it will return the caller's
nr_range. If it failed to merge it will call add_range() and then
will return the caller's nr_range if it ran out of space.
So the caller cannot determine from the return value whether or not
the range was added.
Or something. This is an advantage of actually documenting code -
it makes people think about such things.
- The main structure seems just wrong, or at least inappropriate. Should be
struct range {
/* Number of ranges presently at *ranges */
unsigned nr_ranges;
/* Maximum number of ranges storable at *ranges */
unsigned max_ranges;
struct {
u64 start;
u64 end;
} *ranges;
};
Or similar.
- I can't be bothered working out what subtract_range() and
clean_sort_range() are supposed to be doing, so I didn't look at
them.
c'mon guys, we can do better than this.
On 06/07/2010 03:49 PM, Andrew Morton wrote:
> On Sat, 5 Jun 2010 21:32:15 +0200 (CEST)
> Geert Uytterhoeven <[email protected]> wrote:
>
>> Remove duplicate definition of ARRAY_SIZE(), which was never used anyway.
>>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> ---
>> kernel/range.c | 4 ----
>> 1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/range.c b/kernel/range.c
>> index 74e2e61..471b66a 100644
>> --- a/kernel/range.c
>> +++ b/kernel/range.c
>> @@ -7,10 +7,6 @@
>>
>> #include <linux/range.h>
>>
>> -#ifndef ARRAY_SIZE
>> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> -#endif
>> -
>> int add_range(struct range *range, int az, int nr_range, u64 start, u64 end)
>> {
>> if (start >= end)
>
> <discovers range.c>
>
> That's not terribly great code, sorry.
>
> - The names are all wrong. Should be range_add(),
> range_add_with_merge(), range_subtract(), etc.
>
> - It's completely undocumented!
>
> - It's linked into every vmlinux in the world, many of which won't use it
> afacit.
>
> - The return value from add_range() is a bit odd. I guess callers must do
>
> if (add_range(..., ..., nr_range, ..., ...) == nr_range)
> error()
>
> - What does the identifier "az" mean?
>
> - `az' and `nr_range' should be unsigned types. That would make the
> "Out of slots:" check non-buggy.
>
> - The return value from add_range_with_merge() is unusable! If it
> did a merge into the final range it will return the caller's
> nr_range. If it failed to merge it will call add_range() and then
> will return the caller's nr_range if it ran out of space.
>
> So the caller cannot determine from the return value whether or not
> the range was added.
>
> Or something. This is an advantage of actually documenting code -
> it makes people think about such things.
>
> - The main structure seems just wrong, or at least inappropriate. Should be
>
> struct range {
> /* Number of ranges presently at *ranges */
> unsigned nr_ranges;
> /* Maximum number of ranges storable at *ranges */
> unsigned max_ranges;
> struct {
> u64 start;
> u64 end;
> } *ranges;
> };
>
> Or similar.
>
> - I can't be bothered working out what subtract_range() and
> clean_sort_range() are supposed to be doing, so I didn't look at
> them.
>
> c'mon guys, we can do better than this.
will work on using lmb to replace range.c later after lmb for x86 is in tip and linux-next.
Thanks
Yinghai