DevHeads.net

C++ help needed fixing VXL on 32 bit architectures

Hi everyone,

After spending quite a bit of time fixing VXL to build, I've now run
into errors with it building on 32 bit arches.

Unfortunately, I don't foresee myself having enough cycles in the near
future to debug the C++ bits to see what's happening here, and while I
have filed a ticket upstream, they seem to be even busier than us (all
issues seem to get a "A PR would be welcome" response)

<a href="https://github.com/vxl/vxl/issues/638" title="https://github.com/vxl/vxl/issues/638">https://github.com/vxl/vxl/issues/638</a>

Any chance any C++ ninjas here would have some time to look into this
please? A PR fixing the issue upstream would be absolutely fantastic.

Comments

Re: C++ help needed fixing VXL on 32 bit architectures

By John Reiser at 05/27/2019 - 19:57

Independent of that particular issue, it is hard to believe the claim
"vxl: A multi-platform collection of C++ software libraries ...".
They're not making a good-faith effort to be portable.
The first hint is that "-Werror" (turn all warnings into errors)
has not been used when compiling.

There are so many basic portability errors, such as:

===== portability error 1

v3p/netlib/triangle.c:219: #define TRIANGLE_PTRINT size_t

Using #define is silly when typedef is suitable.

v3p/netlib/triangle.c:3685:23: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Wformat=]
printf("triangle x%lx with orientation %d:\n", (TRIANGLE_PTRINT) t->tri,

"%zx" solves this problem and has been available for many years.

===== portability error 2

v3p/clipper/clipper.cpp:722:34: warning: 'void* memset(void*, int, size_t)' clearing an object of non-trivial type 'struct ClipperLib::TEdge'

Learn from the compiler; it is smarter than this programmer.
(Example: if there are any 'virtual' functions [now or later!],
then using memset zaps the vtable pointer.)

====== portability error 3

core/vil1/vil1_memory_image_impl.cxx:210:63: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
return property_value ? (*(bool*)property_value) = true : true;

This is unmaintainable, especially when not explained in a comment.

===== portability error 4

core/vil1/file_formats/vil1_pnm.cxx:360:11: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if ((t&=7)==0) *++ib=0; int a; (*vs_) >> a; if (a) *ib|=static_cast<unsigned char>(1<<(7-t)); }

Such code is an abomination for lack of clarity.
Also, the preceding line
for (int x=0,t=0; x<xs*components_; ++x,++t) {
suggests that (7 < t) is a possibility, so (1<<(7-t)) will be undefined,
and the run-time bug will be hard to find.

=====

Re: C++ help needed fixing VXL on 32 bit architectures

By Roberto Ragusa at 05/28/2019 - 08:31

On 5/28/19 2:26 PM, Roberto Ragusa wrote:
0..7, sorry for mistyping.

Re: C++ help needed fixing VXL on 32 bit architectures

By Ankur Sinha at 05/28/2019 - 04:59

Hi John,

On Mon, May 27, 2019 16:57:18 -0700, John Reiser wrote:
From what I've seen, VXL seems to be one of those libraries that was
written long long ago and has for most of its life cycle been in
"maintenance mode" to somehow keep it running. For example, my PR to
version the shared objects was only merged last week (!), and the super
spammy nightly commit bot is still active:

<a href="https://github.com/vxl/vxl/issues/624" title="https://github.com/vxl/vxl/issues/624">https://github.com/vxl/vxl/issues/624</a>
<a href="https://github.com/vxl/vxl/pull/626" title="https://github.com/vxl/vxl/pull/626">https://github.com/vxl/vxl/pull/626</a>
<a href="https://github.com/vxl/vxl/issues/97" title="https://github.com/vxl/vxl/issues/97">https://github.com/vxl/vxl/issues/97</a>

Most of this is probably because the core dev community is extremely
stretched. They don't have the cycles to make required updates, which,
for example, is why VXL still uses a dcmtk version from 2002 (and we
have no choice but to bundle it):
<a href="https://github.com/vxl/vxl/issues/550" title="https://github.com/vxl/vxl/issues/550">https://github.com/vxl/vxl/issues/550</a>

So, if you do have the time, please file these issues (or PRs if you
have even more time). That'll at least make them aware of them and maybe
it'll encourage others to help the dev team too.

I'd like to look into all of this, but as soon as I get VXL built, I've
got to work on all the tools that depend on it and are currently broken
also.

Re: C++ help needed fixing VXL on 32 bit architectures

By Jerry James at 05/27/2019 - 16:54

On Mon, May 27, 2019 at 10:14 AM Ankur Sinha <sanjay. ... at gmail dot com> wrote:
It looks like the first few errors, at least, were fixed the day after
the last release, in this commit:

<a href="https://github.com/vxl/vxl/commit/c3fd27959f51e0469a7a6075e975f245ac306f3d" title="https://github.com/vxl/vxl/commit/c3fd27959f51e0469a7a6075e975f245ac306f3d">https://github.com/vxl/vxl/commit/c3fd27959f51e0469a7a6075e975f245ac306f3d</a>

You might try adding that as a patch and see if that is sufficient, or
if there are more problems.

Regards,

Re: C++ help needed fixing VXL on 32 bit architectures

By Jerry James at 05/27/2019 - 17:33

On Mon, May 27, 2019 at 2:54 PM Jerry James < ... at gmail dot com> wrote:
I did a local mock build for i386 with that patch, and it succeeded. FYI.

Re: C++ help needed fixing VXL on 32 bit architectures

By Ankur Sinha at 05/28/2019 - 04:31

Dear Jerry,

On Mon, May 27, 2019 15:33:42 -0600, Jerry James wrote:
Ah! Thanks so much! This turned out a lot simpler than I'd feared. I
hadn't even begun wading through the commit history yet.

I'll go patch-build-update now.