Re: [Gpsbabel-code] MS Visual Studio

[Robert L. <rob...@gp...>]

Thanx.  Applied.  More below.


On Sat, Jul 13, 2013 at 8:01 PM, Gerhard Olsson <ger...@gm...>wrote:

> * tpo.cc - use struct instead of parallel arrays
> While there are embarrassing reasons for the latest failure (xfree before
> out of scope...), I still could not get test pass for the module. I
> therefore rewrote, using a struct (did not see a benefit in a class here).
> Minor optimization (adding variables instead of a weird sprintf->scanf,
> decreasing memory usage slightly) too.
>

Agreed.  There are better ways still to write this block of code, but you
left it better than you found it, so yay for that.


> Note1: I have not used the testo previously, there are a lot of errors
> when validating, most due to rounding. Is this platform specific?
>

Probably.   testo (and its more masochistic brother 'vtesto' which runs
everything through valgrind) run without issue on Mac and a variety of
Linux mutants.  There are some times when we jump through hoops to avoid
rounding issues or deal with formats that are inherently lossy and the
comments in testo.d/tpo.test show this is likely one of those formats.  In
fantasy land, I'd like testo to run on Windows, but I can't say that's been
exercised in many years.  Olaf used to do it from time to time, but the
burden of building up a cygwin environment so you had a sensible shell
always made Windows problematic for us.   At one time, we used to have a
tool that tried to machine parse the test scripts and write a big old batch
file but that worked badly.


> * MSVC update
> Note that GPSBabel.vcproj was renamed to GPSBabel.vcxproj but the patch do
> not handle that.
> In addition are the "msvc7" files removed, I have no possibility to fix
> them.
> Changes:
> * Only MSVS2012 supported now. If there is a real need for other versions,
> that could be added. (but there seem to be few users)
>

Agreed.


> * Revised the README.msvc file
> * Setup for cc (files renamed)
> * Added some new formats missing
> * Added new Qt paths
>

It's unfortunate that has to be a constant, but OK.

As an aside, if you run qmake, it should read GPSBabel.pro (which contains
little more than a list of source files) and hock up an MSVC project file;
you don't have to actually use Qt Creator.   Again, it may be slightly out
of date from time to time (the XCode build has this same issue) as it's not
the primary development mode, but it should be further away from the
details of the build to just add a new format.cc from time to time.



> * Rewrote the file, reduced the size considerably (as options were added
> per file, some were incorrect).
> * The different configurations (Debug/Release/Unicode) shares
> configuration where possible. The Release/Unicode configurations did not
> work before
> * Set parallel build as default
>
> Note2: The problems with providing the patch should be simplified by using
> Mercurical or git instead of SVN. As a user without SVN commit rights, this
> would simplify. I sure understand if you so not want to switch though...
>

My hand may be forced on hg or git, but after only recently moving from
CVS, I don't really have burning desires to move again.


> Note3: Renaming jeeps/gpsutil.cc would be good, there are special handling
> for the file right now. MSVS generally do not like duplicate names.
>

That's kind of lame, but easy enough to work around.  Done. I made a blind
change to the MSVC in the hope of not messing up your fresh work.

Note4: I would prefer to remove the UNICODE configuration, let the user add
> that manually as done in normal build.
>

While Google uses the MSVC compiler to build the GPSBabel that's in Earth,
they don't use the MSVC build system.  So while we have to keep the
XML_UNICODE stuff in the source, I wouldn't veto removing it from the MSVC
build files.


> Note5: I suggest removing make target msvc-build as it does not use all
> settings anyway, will not work well. It is possible to use the MSVS project
> with msbuild instead, the project is used then.
> /cygdrive/c/Windows/Microsoft.NET/Framework/v4.0.30319/MSBuild.exe
> msvc/GPSBabel.vcxproj
>
>
> * shapelib HAVE_CONFIG_H
> Will not compile without a config.h
> Again, this is a situation that is not standard. Another solution is to
> remove the define from the 4 other files it appears in.
>

This is indeed the intersection of uncommon paths.  If you're not using
configure/make, you don't HAVE_CONFIG_H and it's up to you to manually
-DWHATEVER in the build.

Still, I think we're about there.  Thanx for sticking through it with.
RJL



>
>
>
> On Sat, Jul 13, 2013 at 1:48 AM, Robert Lipe <rob...@gp...>wrote:
>
>> Still not right, but I've gotta go for the night.
>>
>> Program received signal EXC_BAD_ACCESS, Could not access memory.
>> Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
>> 0x000000010008bf88 in tpo_process_tracks () at tpo.cc:574
>> 574      style_color[ii][xx] = (int) gbfgetc(tpo_file_in);
>> (gdb) p style_color
>> $1 = (int **) 0x1007117e0
>> (gdb) p style_color[0]
>> $2 = (int *) 0x0
>> (gdb) p style_color[0][0]
>> Cannot access memory at address 0x0
>>
>> This is a good example of the kind of code I'm really hoping to get rid
>> of.  Why is there not a struct/class that contains the name style width,
>> and dash that is then built into an array instead of these weird three
>> parallel arrays?
>>
>>
>>
>> On Fri, Jul 12, 2013 at 6:33 PM, Gerhard Olsson <ger...@gm...
>> > wrote:
>>
>>> Last update for tonight
>>>
>>> * tpo.cc update
>>> sizeof(int) != 1 (so style_color was too small)
>>> Added xfree() (phew...)
>>> switched order for xcalloc(number, size) (it seems like the argument
>>> order is switched much in the code, but for dynamic arrays it looks better
>>> with the xcalloc order anyway)
>>>
>>>
>>> On Sat, Jul 13, 2013 at 12:57 AM, Robert Lipe <rob...@gp...>wrote:
>>>
>>>> Mostly applied.  The change in tpo.cc introduces a crash that looks
>>>> like a bad index value in style_color.  I'm not sure I'll get to it
>>>> tonight, so could you please look closer at that one?  The double pointer
>>>> indirection is funky, but I think you missed the multiplier of
>>>> TRACKNAMELENGTH.
>>>>
>>>> ...oh, and please sprinkle some xfrees() on these when we leave scope.
>>>>
>>>> Thanx,
>>>>
>>>>
>>>>
>>>> On Fri, Jul 12, 2013 at 5:40 PM, Gerhard Olsson <
>>>> ger...@gm...> wrote:
>>>>
>>>>> * QString toAscii() removed in Qt5.1
>>>>> Replaced use of toAscii() with toLatin1() so Qt5.1 can be used with
>>>>> GPSBabel (commandline, GUI not tested)
>>>>> Note that gtm.cc file contains "datum" patch too.
>>>>>
>>>>> (last code patch in the planning right now)
>>>>>
>>>>>
>>>>> On Sat, Jul 13, 2013 at 12:17 AM, Gerhard Olsson <
>>>>> ger...@gm...> wrote:
>>>>>
>>>>>> * gtm.cc MSVC compiler limit
>>>>>> Converted "else if" structure to array with the following perl
>>>>>> snippet, so not hand coded
>>>>>>  perl -ne 'BEGIN{$i=-1; $n=0} $t=0; if(/\bn\s*\<\s*(\d+)/){$n2=$1;}
>>>>>> if(/indatum\s*=\s*(-?\d+).*\/\*\s*(.*)\*\//){$i=$1;$c=" : $2";
>>>>>> $t=1;}elsif(/\{\s*\}/){$i=-1;$c=""; $t=1;} if($t){print "
>>>>>> ";while($n<$n2){print " $i,";$n++;}print " // < $n2$c\n"}' gtm.c.snippet
>>>>>>
>>>>>>  * tpo.cc variable length array
>>>>>>
>>>>>>
>>>>>> On Fri, Jul 12, 2013 at 10:45 PM, Gerhard Olsson <
>>>>>> ger...@gm...> wrote:
>>>>>>
>>>>>>> * Updated patch for gbtypes.h
>>>>>>> Removed special handling for MSC
>>>>>>> It would make sense if all gb* types were replaced.
>>>>>>> (My comment regarding mtk_locus was that the module does not use the
>>>>>>> "standard" gb* types and that a change was needed.)
>>>>>>>
>>>>>>> * crt_util.c XML_UNICODE compile
>>>>>>> (not sure if if last change is OK, if this configuration used? The
>>>>>>> MSVC setup has it, can it be removed?)
>>>>>>>
>>>>>>> * geo.cc did not compile without HAVE_EXPAT
>>>>>>> It is not likely that GPSBabel is used without expat though....
>>>>>>> (I happened to compile without the define once)
>>>>>>>
>>>>>>>
>>>>>>> (other minor changes still pending)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jul 12, 2013 at 7:40 PM, Robert Lipe <
>>>>>>> rob...@gp...> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>  * Type setup for MSVC. int and long are both 32bit in MSVC, both
>>>>>>>>> 32/64 bit.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That's actually a very common case.
>>>>>>>>
>>>>>>>> Since *most* C89 implementations actually offered a sane <stdint.h>
>>>>>>>> and C99 and C++ (all) require it, I wonder if we could toss the exception
>>>>>>>> for MSVC in that file completely  and let gbint32 be a typedef for int32_t.
>>>>>>>>   (This file...or its equivalent in defs.h back int he old days... used to
>>>>>>>> be really ugly, but the fixed size type things are pretty institutional in
>>>>>>>> the industry these days.
>>>>>>>>
>>>>>>>> If you just whack the if MSC_VER block and let it use the 'normal'
>>>>>>>> case, does it stay happy these days?
>>>>>>>>
>>>>>>>> At least MSVS2012 has a stdint.h though. mtk_locus.cc uses uint32_t
>>>>>>>>> directly, so that definition is needed (or mtk_locus to be redefined).
>>>>>>>>>
>>>>>>>>
>>>>>>>> Why would that not be safe?  Surely uint32_t isn't a 64 bit size on
>>>>>>>> MSVC ever, is it?
>>>>>>>>
>>>>>>>> * lowrance* uses round() and lround(), not included in MS pre C99
>>>>>>>>> implementation of math.h.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Sigh.  OK.  They're actually right on this one, but they sure do us
>>>>>>>> no favors.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Using si_round instead (GPSBabel internal, will not handle a few
>>>>>>>>> cases exactly the same but should be good enough for development). This
>>>>>>>>> change could be applied in defs.h instead.
>>>>>>>>> (emitt warnings for release builds in MSVC?)
>>>>>>>>>
>>>>>>>>> * xmlgeneric: Adding const to declaration to be same as in
>>>>>>>>> definition.
>>>>>>>>> While I would expect any compiler to warn about this, this fails
>>>>>>>>> in the MSVC linker
>>>>>>>>>
>>>>>>>>> Pending:
>>>>>>>>>  * A change in gtm.cc (discussed above)
>>>>>>>>>  * tpo.cc tpo_process_tracks() (track_style_count must be constant
>>>>>>>>> in array creation) pending. This is in my view a relevant change, even if
>>>>>>>>> it is annoying that MSVC is different.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'm a litlte surprised to see VLA used here.  Normally I catch
>>>>>>>> those in review.  That should probably be just a plain ole dynamically
>>>>>>>> allocated array.
>>>>>>>>
>>>>>>>> [ scowls.  types ] Fixed.
>>>>>>>>
>>>>>>>> Thanx, all committed.
>>>>>>>>
>>>>>>>> RJL
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>   * Changes to the project. Will be submitted for MSVS2012.
>>>>>>>>> Should not be a major job to change to MSVS2010, but there are a few more
>>>>>>>>> changes to older versions.
>>>>>>>>>  * Make it possible to use Qt 5.1 for command line GPSBabel
>>>>>>>>> without breaking 4.x, as the replacing function works the same in 5.1 and
>>>>>>>>> 4.6. (none exists in 3.3 though?) The change required is to replace
>>>>>>>>> toAscii() with toLatin1(), which is wanted anyway I believe. Affects 5
>>>>>>>>> files (so about the same time fixing as it takes to discuss).
>>>>>>>>>
>>>>>>>>> /Gerhard
>>>>>>>>>
>>>>>>>>> On Fri, Jul 12, 2013 at 5:12 AM, Robert Lipe <
>>>>>>>>> rob...@gp...> wrote:
>>>>>>>>>
>>>>>>>>>> On Thu, Jul 11, 2013 at 8:57 PM, Gerhard Olsson <
>>>>>>>>>> ger...@gm...> wrote:
>>>>>>>>>>
>>>>>>>>>>> Should MSVS be considered as retired or should I send some
>>>>>>>>>>> patches to get it running?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Once or twice a year, someone asks about it. It's not strategic
>>>>>>>>>> to ME, but if it's useful to you, I'll check in patches and keep it in the
>>>>>>>>>> tree.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> What version of MSVS should be used? I only have MSVS2012
>>>>>>>>>>> (Express) myself and will have a hard time testing something else.
>>>>>>>>>>> (Something like CMake would help here.)
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> QMake would, too.  I checked in a GPSBabel.pro.  It's probably
>>>>>>>>>> slightly out of date; certainly less so than MSVC.  I've considered a
>>>>>>>>>> hybrid approach where I could keep Makefile.in for doing all the heavy
>>>>>>>>>> lifting that QMake can't do and call qmake on GPSBabel.pro to build the
>>>>>>>>>> executable.
>>>>>>>>>>
>>>>>>>>>> I think in the days when MSVC updates were a zillion dollars
>>>>>>>>>> there was value in worrying about last year's version or a version back.
>>>>>>>>>>  Now that MSVC and updates are free for projects like this, I'd worry much
>>>>>>>>>> less about breaking last year's MSVC.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Note: There are still some peculiarities with the MS c++
>>>>>>>>>>> compiler....
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Oh, the shock.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>  gtm.cc set_datum() too many nested blocks" (that should be
>>>>>>>>>>> written with an array in my opinion, but still a non standard deviation,
>>>>>>>>>>> C++ spec say 256 levels).
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Wow, that function *is* bizarre.   Rather than returning an int,
>>>>>>>>>> it sets a global.  Rather than
>>>>>>>>>> if (foo)
>>>>>>>>>>   return bar
>>>>>>>>>> if (more foo)
>>>>>>>>>>    return more bar
>>>>>>>>>>
>>>>>>>>>> ...or a switch or a hash lookup (admittedly much easier now with
>>>>>>>>>> QHash than in raw C) or just about anything else.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> What are the plans for Qt (version redistribution etc)?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Since we're already bundling Qt in the GUI - and have for about
>>>>>>>>>> five years - it should already be in the packaging and build systems, etc.
>>>>>>>>>>   My Windows cross compiles have been statically linking libQt so we remain
>>>>>>>>>> relatively self contained.  (Yes, that means we don't benefit from sharing
>>>>>>>>>> the copy between the GUI and the command line executable.  In 1988, I would
>>>>>>>>>> have stressed out about such things.)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Qt may require some tweak when setting up a project.
>>>>>>>>>>> As only 5.1 is enabled for VS2012, some manual tweaks to  Qt or
>>>>>>>>>>> GPSBabel is required to compile (QString toAscii() is removed for instance.)
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> That's going to be awkward.  I'd really hoped to stay in 4.x
>>>>>>>>>> where the compatibility across versions is pretty good as long as you don't
>>>>>>>>>> intentionally use new things, for example..  We'll probably all be retired
>>>>>>>>>> before CentOS updates to Qt5.1 (see last week's gnashing of teeth).  All of
>>>>>>>>>> that toAscii stuff really is just a crutch to prop things up while moving
>>>>>>>>>> from C Strings to QString.  Frankly, most of it is done pretty mechanically
>>>>>>>>>> and at huge slash-and-burn rate.  All that silliness in mapsend.cc would be
>>>>>>>>>> much more naturally written with a QChar for the comparisons and than just
>>>>>>>>>> call toLatin1() on it in the gbfwrite, knowing that this value isn't EVER
>>>>>>>>>> anything other than plain ole ascii.  I remember looking at our GUI with
>>>>>>>>>> Qt5 and there is some stuff in it that was incompatible, but I chose to
>>>>>>>>>> flee instead of fight.
>>>>>>>>>>
>>>>>>>>>> While I'd love to have a hand in the Qt move of our core, given
>>>>>>>>>> that version constraint for MSVC, I'd have a tough time encouraging you to
>>>>>>>>>> tackle MSVC if your goal is just to get an executable.  (Well, if the
>>>>>>>>>> suffering is anything more severe than that toAscii example which I could
>>>>>>>>>> have probably just eliminated in the time I spent explaining it.)  The 4.x
>>>>>>>>>> QtBuilder approach will probably be a smoother road unless you have deep,
>>>>>>>>>> burning desire for MSVC.  I don't actively use QTCreator, but can do it at
>>>>>>>>>> any time and that'll get your work flow a heck of a lot closer to mine
>>>>>>>>>> without full-blown cross compilation or cygwin or battling MSVC.
>>>>>>>>>>
>>>>>>>>>> ISTR there was something stupid in QtCreator where it flipped out
>>>>>>>>>> that we had a file in jeeps/ and one in the main source that shared a name
>>>>>>>>>> (gpsutil.cc?) and that required some intervention.  I considered that a bug
>>>>>>>>>> in QtCreator but never really chased it.  I remember symlinking or renaming
>>>>>>>>>> the one in jeeps/, I think.   But Creator was working for me at one point.
>>>>>>>>>>  I'd expect some missing source files, but you're familiar with that game.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------------------------
>>>>>>>>> See everything from the browser to the database with AppDynamics
>>>>>>>>> Get end-to-end visibility with application monitoring from
>>>>>>>>> AppDynamics
>>>>>>>>> Isolate bottlenecks and diagnose root cause in seconds.
>>>>>>>>> Start your free trial of AppDynamics Pro today!
>>>>>>>>>
>>>>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>>>>>>>>> _______________________________________________
>>>>>>>>> Gpsbabel-code mailing list  http://www.gpsbabel.org
>>>>>>>>> Gps...@li...
>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> See everything from the browser to the database with AppDynamics
>>>>> Get end-to-end visibility with application monitoring from AppDynamics
>>>>> Isolate bottlenecks and diagnose root cause in seconds.
>>>>> Start your free trial of AppDynamics Pro today!
>>>>>
>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>>>>> _______________________________________________
>>>>> Gpsbabel-code mailing list  http://www.gpsbabel.org
>>>>> Gps...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
>>>>>
>>>>>
>>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> See everything from the browser to the database with AppDynamics
>>> Get end-to-end visibility with application monitoring from AppDynamics
>>> Isolate bottlenecks and diagnose root cause in seconds.
>>> Start your free trial of AppDynamics Pro today!
>>>
>>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>>> _______________________________________________
>>> Gpsbabel-code mailing list  http://www.gpsbabel.org
>>> Gps...@li...
>>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
>>>
>>>
>>
>