[mkgmap-dev] methods to write signed / unsigned integers

Tue Feb 20 10:52:11 GMT 2018

Hi Gerd

The final bit patterns written into the IMG will be the same regardless
of using putUx or putSx, but the Garmin device or map renderer will
interpret each as either signed or unsigned, depending on where it is
in the IMG. If we write, say a value of 200 into a byte, where this is
interpreted as a signed by the device, we just have a possibly
difficult-to-diagnose map failure. Similarly writing, say, -1, to
something that will be interpreted as unsigned will cause a problem.

Having different methods with relevant assertions allows these errors
to be caught early on. It also clarifies the code.

Given that nearly all the int writes in imgfmt are going to be changed,
it seems sensible to do it as best as possible.

Also, in imgfmt, there is code to read back the IMG structures. This
has to known if it should use signed or unsigned read methods. Often,
quite close in the code, there will be the output calls for the same
items, so having: 
  someFld = reader.get1S();
  ...;
  writer.put1S(someFld) 
looks difficult to argue with. 

I don't understand your comment:
  "... we can use int even if IMG format uses a single byte, since we
don't have huge amounts of header structures."
It is only during the method call (putxx()) or return (getxx()) that
ints will be in use rather than byte/char

Regards
Ticker

On Tue, 2018-02-20 at 09:15 +0000, Gerd Petermann wrote:
> Hi Ticker,
> 
> please explain why you think that this is important. I like the idea
> that I don't have to care about the
> write method, I only have to make sure that the fields can hold the
> value ranges. In most cases we can
> use int even if IMG format uses a single byte, since we don't have
> huge amounts of header structures.
> 
> Gerd
> 
> ________________________________________
> Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im Auftrag
> von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> Gesendet: Montag, 19. Februar 2018 10:19:56
> An: Development list for mkgmap
> Betreff: Re: [mkgmap-dev] methods to write signed / unsigned integers
> 
> Hi
> 
> I think it is important to indicate if a signed or unsigned is being
> written so that the range can be asserted:
> 
> eg:
>         public void putU1(int val) {
>                 assert val >= 0 && val <= 255 : val;
>                 buf.put((byte)val);
>         }
> and
>         public void putS1(int val) {
>                 assert val >= -128
> && val <= 127 : val;
>                 buf.put((byte)val);
>         }
> 
> 
> Ticker
> 
> 
> On Sun, 2018-02-18 at 21:37 +0000, Steve Ratcliffe wrote:
> > Hi Gerd, Ticker
> > 
> > For ImgFileWriter I think we should just have put1(int), put2(int),
> > put3(int)
> > and put4(int) and remove put(byte), putChar(char) and putInt(int).
> > 
> > So each method takes an int, so no casting is needed.  There is no
> > difference between writing unisigned and signed for any value
> > that fits into an int.
> > 
> > To write unsigned values greater than 2G then technically you
> > need a long, so an unsigned version could be added as a
> > default method on the interface put4u(long val) rather than
> > having to implement across multiple implementations.
> > 
> > Although I don't think it is necessary if you want to add
> > signed/unsigned methods that range check the value, then
> > again add them as default method on the interface.
> > 
> > I've attached a patch that implements this.
> > 
> > Reading is different, we do need signed/unsigned versions.
> > I'd suggest that again have get{1,2,3,4} and remove getChar and
> > getInt special cases, and then get1u() etc for unsigned results.
> > All of these returning int probably, depending on what results
> > in the best looking code.  I have not done a patch for that.
> > 
> > ..Steve
> > 
> > > we have various methods to write an integer with 1, 2, 3, or 4
> > > bytes to an img file.
> > > I always feel unsure what method to use because none of them
> > > makes
> > > clear what happens with negative values.
> > > 
> > > Besides that some of the existing routines seem to throw
> > > misleading
> > > exceptions,
> > > e.g. FileBackedImgFileWriter seems to assume that it is only used
> > > for the mdr tmp file and creates texts like this:
> > > throw new MapFailedException("could not write3 to mdr tmp file");
> > > throw new MapFailedException("could not write put3 to mdr tmp
> > > file");
> > > 
> > > Only the javadoc for put1() and put2() tell me the range (0..255
> > > or
> > > 0..65535) .
> > > 
> > > If I got that right put3() allows negative values, so I think it
> > > is
> > > NOT 0..16777215 but -8388608 .. 8388607 ?
> > > 
> > > I'd like to improve the readability of the code, but I don't want
> > > to mess up anything.
> > > Would it be possible to add comments to all the methods?
> > > 
> > > Gerd
> > > 
> > > 
> > > 
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev at lists.mkgmap.org.uk
> > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > 
> > 
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev