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

Fri Mar 16 09:38:13 GMT 2018

Hi Gerd

Looking at the Displayer and the DisplayItem methods, I think it would
be quite a lot of work for little benefit to change it all to have the
various 1/2/3/4 s/u type methods. It would need all the variants on
setBytes as well. 

Unless we introduce differently named setBytes() methods, we still need
the casts as in item.setBytes((char)reader.get2u())

All the redundant masking should be removed

Also any obvious uses of signed when should be unsigned get, along the
other errors that have been noticed

I'll can do another pass over the code with these changes and send you
a combined path if you want, but I feel it would be simpler and saver
to apply the current patch at the same time as you apply the main
mkgmap patch and make these changes as a new version

Ticker 

On Fri, 2018-03-16 at 08:22 +0000, Gerd Petermann wrote:
> Hi Ticker,
> 
> yes, I think display tool needs more work. You commented a
> I think the method names in Displayer.java should be changed so that
> they are similar to the corresponding reader methods.
> Example:
>         public int int3Value(String text)
> would be
>         public int int3uValue(String text)
> 
> I see some places where you replaced
>                 item.setBytes(reader.getChar());
> by
>                 item.setBytes((char)reader.get2u());
> I think the cast to char should be removed.
> 
> I also see code like this:
>                                 c = item.setBytes(reader.get2u()) &
> 0xffff;
> I think the & 0xffff is obsolete and confusing.
> 
> I am pretty sure that some lines in MdrDisplay are wrong (not because
> of your patch).
> I think all places where get3s() is used need a closer look. We might
> have to change them to get3u().
> For example:
>  int record = reader.get3s();
> off = item.setBytes3(reader.get3s());
> Record numbers and offsets are normally not negative. I'll have a
> closer look today.
> 
> I also agree that this snippet from NodConvert looks wrong:
> 		} else if (restrbytes == 3) {
> 			size = reader.get2u() & 0xffffff;  // %%% think
> mistake
> 
> I've not used NodConvert for a long time, it might well be behind the
> last findings reg. NOD.
> 
> Gerd
> 
> 
> ________________________________________
> Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im Auftrag
> von Ticker Berkin <ticker at jagIT.co.uk>
> Gesendet: Donnerstag, 15. März 2018 15:21:25
> An: mkgmap-dev at lists.mkgmap.org.uk
> Betreff: Re: [mkgmap-dev] methods to write signed / unsigned integers
> 
> Hi Gerd
> 
> I've attached the Display patch - this was a quick hack global edit
> and
> could be improved by using the relevant signed/unsigned get methods
> instead of masking...  I'm happy to do this - let me know.
> 
> Yes to DEM changes. Also the nearby asserts can go now because
> put2s()
> will check the range.
> 
>   // don't think needed:
> could be changed to
>   // not needed at moment:
> but I think it is worth leaving these methods (getNs()/putNs())
> commented in the ImgFile interface source.
> 
> Ticker
> 
> 
> On Thu, 2018-03-15 at 10:12 +0000, Gerd Petermann wrote:
> > Hi Ticker,
> > 
> > sorry, did not find the time to look at it until now.
> > At least for DEM this would not be correct, some values should be
> > written with the signed put versions.
> > I've attached a corrected version of the patch.
> > Besides that I'd prefer not to see comments like
> > // don't think needed:
> > 
> > For further testing I'd need the patch for display tool as well.
> > 
> > Gerd
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev