Re: [ext2resize] [PATCH] Endless loop in ext2_move_blocks()?

[Andreas D. <ad...@cl...>]

On Mar 16, 2006  08:21 +0100, Petter Reinholdtsen wrote:
> As i is unsigned, it will always be 0 or higher, and this the else
> block is an endless loop.
> 
> What is the best way to fix this?  My suggestion is to run i from num
> to 0 instead of trying to run from num-1 to -1:

Actually, loop should be from num-1 to 0 (i.e. copy 'num' blocks in reverse
order), and your change would make this from num to 1.  The change is
correct, just your description is slightly wrong...

> --- ext2resize-1.1.19.orig/src/ext2.c
> +++ ext2resize-1.1.19/src/ext2.c
> @@ -131,8 +131,8 @@
>                 for (i = 0; i < num; i++)
>                         ext2_copy_block(fs, src + i, dest + i);
>         else
> -               for (i = num - 1; i >= 0; i--)
> -                       ext2_copy_block(fs, src + i, dest + i);
> +               for (i = num; i > 0; i--)
> +                       ext2_copy_block(fs, src + i - 1, dest + i - 1);

This seems fine.

> Can the default build be changed to include -W in addition to -Wall?
> It show quite a lot of signed/unsigned issues, and I suspect it is
> better to expose those to more people.  I suggest adding code like
> this to configure.in just before the "dnl Checks for libraries." line.
> 
>   if eval "test x$GCC = xyes"; then
>      CFLAGS="$CFLAGS -W -Wall"
>   fi

That really depends on how many such errors there are, and how easy or
hard it is to fix them.  I don't disagree in principle, but it would
be bad to miss important warnings in a flood of unimportant ones.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.