Incorrect cast from int to pointer

[Michi Henning]

./GpuDeviceVertex.cpp: In member function 'int nux::GpuDevice::DrawIndexedPrimitive(nux::ObjectPtr<nux::IOpenGLIndexBuff
er>, nux::ObjectPtr<nux::IOpenGLVertexDeclaration>, nux::PRIMITIVE_TYPE, int)':
./GpuDeviceVertex.cpp:158:7: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

The code is:

      CHECKGL(glVertexAttribPointer(shader_attribute_location,
        vtxelement.NumComponent,
        vtxelement.Type,
        GL_FALSE,
        vtxelement.stride_,
        (void*)vtxelement.Offset));

vtxelement.Offset is of type int, so the compiler rightfully complains about a cast from a 4-byte int to an 8-byte void*.

It looks like something is missing here, such as a stream pointer to which the offset should be added, but it's not clear to me what the correct fix is. But, as it stands, it's certainly incorrect.

[Michi Henning (michihenning)]

Fixed on trunk by revisiont 676.

[Timo Jyrinki (timo-jyrinki)]

As this is targeted for 3.0 series as well, I tried cherry-picking the two commits but they have conflicts on lp:nux/3.0. I assume for now 3.8 can be released without this bug fix.

[Michi Henning (michihenning)]

If you want to back-port the fix, here it is:

      CHECKGL(glVertexAttribPointer(shader_attribute_location,
        vtxelement.NumComponent,
        vtxelement.Type,
        GL_FALSE,
        vtxelement.stride_,
        (void*)&vtxelement.Offset));

Note the & added on the last line.

[Łukasz Zemczak (sil2100)]

Sadly, build-time warnings are not SRU-able, so setting it to "Won't Fix" for 3.0.

[Michi Henning (michihenning)]

I don't know what "SRU-able" means. The warning indicates a 100% guaranteed core dump. Seems like that is worth fixing?

[Łukasz Zemczak (sil2100)]

SRU-able means that a bug can be included in an SRU. And commits that can be included in an SRU have to be either bug fixes (crashers) or really important feature improvements. If this is indeed a potential crasher fix, then we should change the bug title to indicate so, so that no one mistakes it for something not really fitting and SRU. Also, the trunk commit was really really large, including a lot of refactoring, which is also not really SRU material. If the fix that's worth backporting a one-liner as noted in the comment, then maybe this can be considered an SRU bug worth including in 3.0 - if it indeed would prevent a core dump.
And we need to ensure that no commits that are unnecessary land in the SRU branches - otherwise package uploads will be rejected, as what happened 2 times in the course of 2 weeks.

We also didn't notice a core dump with this code before, even though this particular faulty line is in nux since revision 513, which is one year old already. It's a VERY long time.

There was also no merge request already prepared, so I was only basing my opinions on the 4.0 fix - which is a big refactoring merge request that would rather get rejected by distro. So, if you think this fix is important, the crash probable and change small, then feel free to submit a merge request. We can always switch the bug back from 'Won't Fix' to 'Fix Committed' with no problem.

[Stephen M. Webb (bregma)]

Fix Released in Nux Nux 4.0.0.