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 <[EMAIL PROTECTED]>: > > 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 <[EMAIL PROTECTED]> > -- Yann Chemin International Rice Research Institute Office: http://www.irri.org/gis Perso: http://www.freewebs.com/ychemin YiKingDo: http://yikingdo.unblog.fr/ _______________________________________________ grass-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/grass-dev
