[GRASS-dev] Re: i.modis.qc

[Yann Chemin]

Hi again Glynn,

I checked my compilation flags, they are:
CFLAGS="-ggdb -Wall -Werror-implicit-function-declaration"

and on my compiler there is not any warning coming up on compilation
of i.modis.qc.

this is what gcc -v tells:
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian
4.3.2-1' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3
--enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc
--enable-mpfr --enable-cld --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=x86_64-linux-gnu
Thread model: posix
gcc version 4.3.2 (Debian 4.3.2-1)

Yann


2008/10/26 Glynn Clements <glynn at gclements.plus.com>:
>
> Yann Chemin wrote:
>
>> maybe it is wiser indeed to change to ">>=" for platform/compiler
>> stability as it forces the variable to be updated.
>> Somehow it is not necessary on my system (Debian/Sid).
>
> If you want to modify the variable, you need to use >>=. >> just
> returns the shifted value, which is pointless in this particular case.
>
> A statement like:
>
>        x >> 1;
>
> makes no more sense than:
>
>        x + 1;
> or:
>        x * 2;
>
> There's no point evaluating an expression unless you're going to do
> something with the result, e.g. assign it to a variable, or pass it to
> a function which will make use of it.
>
> If you aren't getting warnings, that's probably because your CFLAGS
> doesn't include -Wall or similar. The expression is perfectly valid C,
> so you won't get a warning by default. -Wall enables various optional
> warnings, including warnings for code which is valid but which might
> not do what you expect (e.g. "if (x = 0) ..." is valid, but it's more
> likely to be a typo for "if (x == 0) ...").
>
> However, there is still the question of which variable should be
> updated. E.g. in qc250b():
>
>    qctemp >> 2;                /*bits [2-3] become [0-1] */
>    qctemp = pixel & 0x03;
>
> Changing this to >>=:
>
>    qctemp >>= 2;               /*bits [2-3] become [0-1] */
>    qctemp = pixel & 0x03;
>
> is equally pointless, as you would be modifying qctemp then
> immediately discarding the modified value. Both of the above versions
> are equivalent to just:
>
>    qctemp = pixel & 0x03;
>
> Which leads me to suspect that it's "pixel" which should have been
> shifted, not qctemp.
>
> If you don't understand what this code is supposed to do, please
> remove it. I don't understand what it's supposed to do either, so I
> can only notice things which are obvious mistakes, not anything which
> *could* plausibly be correct but actually isn't.
>
> --
> Glynn Clements <glynn at gclements.plus.com>
>



-- 
Yann Chemin
International Rice Research Institute
Office: http://www.irri.org/gis
Perso: http://www.freewebs.com/ychemin
YiKingDo: http://yikingdo.unblog.fr/