r.external - was: Re: [GRASS-dev] some questions about future development

[Glynn Clements]

Markus Neteler wrote:

> >> this is excellent! Thanks so much. I have tested it but no success yet:
> >
> >> >> Tomorrow, I'll look into creating an r.external module, adding null
> >> >> support, etc.
> >> >
> >> > Okay, done (well, enough to try it out without having to manually hack
> >> > the files; null support is absent).
> >>
> >> [for my convenience I have copied it over to GRASS 6.4.svn locally]
> >
> > Copied what over? r.external won't work unless the corresponding
> > lib/gis changes have also been incorporated, and lib/gis was built
> > with GDAL_LINK=1:
> >
> >> At present, r.external is built automatically, but the code to read
> >> linked maps is conditionalised upon GDAL_LINK, so you need to build
> >> with "make GDAL_LINK=1". If you forget that part, r.external will
> >> work, but attempting to read the map will fail (the cell/fcell files
> >> are zero-length).
> 
> Bingo. I overlooked the GDAL_LINK thing.
> 
> > Your errors are consistent with lib/gis not having the necessary
> > support.
> 
> Right.
> 
> Now r.stats works! Will try to (locally) backport to 6.4 now (for
> monitor tests).
> 
> Thanks again,
> Markus
> 
> PS: Small issue:
> quant.c:293:1: warning: "MIN" redefined
> In file included from /usr/local/include/gdal.h:40,
>                  from
> /home/neteler/grass70/dist.x86_64-unknown-linux-gnu/include/grass/gis.h:32,
>                  from quant.c:286:
> /usr/local/include/cpl_port.h:257:1: warning: this is the location of
> the previous definition
> quant.c:294:1: warning: "MAX" redefined
> /usr/local/include/cpl_port.h:258:1: warning: this is the location of
> the previous definition

Fixed, also the one in fpreclass.c.

I #undef'd any existing definition, as the GDAL version is broken, as
were the versions in quant.c, fpreclass.c, r.external, and r.in.gdal.

Now that I mention it, also see broken versions of MAX() or max() in:

	lib/external/shapelib/shpopen.c
	lib/g3d/tilewrite.c
	raster/r.carve/enforce_ds.c
	raster/wildfire/r.ros/spot_dist.c (*)
	raster3d/base/r3.mask.main.c
	raster3d/base/r3.null.main.c
	raster3d/r3.in.ascii/main.c
	raster3d/r3.in.v5d/main.c
	raster3d/r3.out.ascii/main.c
	raster3d/r3.out.v5d/main.c
	vector/v.in.ogr/main.c

(*) r.ros is broken in the opposite way to the rest.

When defining macros, *always* put parentheses around the expression
and all of the the arguments. IOW:

	#define max(a,b) ((a) < (b) ? (a) : (b))

not (like in r.ros):

	#define max(a,b) (a) < (b) ? (a) : (b)

or (like all of the others):

	#define max(a,b) (a < b ? a : b)

Otherwise, it will do the wrong thing if the arguments are complex
expressions, or the macro is used within a complex expression. E.g.:

	#define max(a,b) (a < b ? a : b)

	max(x & 0xFF, y & 0xFF)
==	x & 0xFF < y & 0xFF ? x & 0xFF : y & 0xFF
==	x & (0xFF < y) & 0xFF ? x & 0xFF : y & 0xFF

[as < has higher precedence than &]

Or:

	#define max(a,b) (a) < (b) ? (a) : (b)

	max(x, y) + 1
==	(x) < (y) ? (x) : (y) + 1
==	(x) < (y) ? (x) : ((y) + 1)

[as + has higher precedence than ?:]

Even if it doesn't actually matter when you write the macro (e.g. 
because the arguments are variables and the macro call is the entire
RHS of an assignment), you never know when someone will add a "+ 1" or
whatever and spend hours figuring out what's wrong. Macros may look
like functions, but they don't behave anything like them.

Also, h2g2 reference:

	#include <stdio.h>
	#define SIX  1 + 5
	#define NINE 8 + 1
	int main(void) {
	    printf("SIX * NINE = %d\n", SIX * NINE);
	    return 0;
	}
=> 42

-- 
Glynn Clements <glynn at gclements.plus.com>