[GRASS5] [bug #3742] (grass) r.out.bin output incorrect compared with r.out.netcdf

[Hamish]

> > > > > > []% r.out.bin -h null=NaN in=srtm out=srtm.grd
> > > > 
> > > > note null=NaN should not work [yet it does! G_parser bug!].
> > > 
> > > G_parser() doesn't actually check that argument values are
> > > legitimate unless the ->options field is non-null.
> > 
> > I didn't know that. Ok, it's just a string.
> > 
> > Should it be testing the string against opt->type? (,opt->multiple)
> 
> Arguably.
> 
> > (ie is there a reason not to do so, or has it just not been done
> > yet?)
> 
> Given that it hasn't been done so far, it's always possible that some
> modules are setting opt->type to TYPE_INTEGER but allowing arguments
> which aren't actually integers. Adding the type-checking would prevent
> such "enhancements" from working.
> 
> I'd say: go for it. If it breaks a module, fix the module (i.e. change
> it to use TYPE_STRING instead).

I came across this the other day with d.grid. In order to take ddd:mm:ss
input for lat/lon grid spacing it sets size= as TYPE_STRING. If this
confuses on the help page or GUI hinting, opt->key_desc can be set.


> > Glynn, I was hoping you would know the answer to the outstanding GMT
> > 
> > header code byteswap question?
> > 
> >     /* Set up Parameters for GMT header */
> >     if (flag.gmt_hd->answer) {
> > ...
> > 	if (flag.swap->answer)
> > 	    TIFFSwabLong((uint32 *) & null_str);
> > 	sprintf(buf, "%d", null_str);
> > ...
> >     }
> > 
> > does this pollute null_str byteswapping later on in the code?
> >  (ie swapping an already swapped variable)
> 
> It looks that way.

ok; I don't know if this swap is actually needed, but will copy to a temp
variable first to isolate any damage.

 
> Also, this:
> 
> 	fprintf(fp_1, "nodata %f\n", null_str);
> 
> is bogus, as null_str is an int, but fprintf will be expecting a
> double.

I fixed that in CVS the other day.

 
> I suggest that null_str should actually be a double, and the
> conversion of the argument to null= should have separate integer/FP
> cases depending upon the input map type.

ok, sounds reasonable.
 

> > or should "swapFlag" be renamed something better?
> 
> Yes. It should be called something like outputLittleEndian or
> outputIntel, as that's what it represents.

ok

> On a related note, the meaning of the -s flag is ill-conceived; the
> output shouldn't depend upon the host's byte-order, only upon the
> options.
> 
> It would be better to have e.g. endian={big,little,host}, so you can
> use endian=big or endian=little to get consistent results regardless
> of the byte order of the host on which you happen to be executing the
> command, or endian=host if you just want whatever will be most
> efficient on the system you're using.

I am going to leave this to someone with more experience with big
endian systems and the ability to test the outcome in a real sense.


Hamish