[no bug] Bug in COGLES2MaterialRenderer

[porcus]

Hi,

there's a bug in COGLES2MaterialRenderer.cpp in both implementations of setPixelShaderConstant:

original code:

Code: Select all

if(index < 0 || UniformInfo[index].location < 0)
return false;

If index is <0 it will crash because of UniformInfo[index].

Please replace it (both times) with (or similar):

Code: Select all

    if(index < 0 || index >= (int)UniformInfo.size())
        return false;
 
    if(UniformInfo[index].location < 0)
        return false;

greetings

[Nadro]

Thanks, I'll fix that.

[Nadro]

There is no bug in the following condition:

Code: Select all

if(index < 0 || UniformInfo[index].location < 0)

It's related to short-circuit evaluation.

Sorry that at first I wrote that I'll fix that, but I didn't precisely check your post

[AReichl]

- If index is <0 it will crash because of UniformInfo[index].
porcus - do you THINK it will crash or HAS it crashed?

Nadro is right - with an OR if the first is "true" the rest can be skipped (Konjunction).
Same with an AND: if the first is "false" the rest can be skipped (Disjunction).

Nevertheless - "if( ... index >= (int)UniformInfo.size() )" maybe also should be checked
for safety. Only god or Nadro know.

[porcus]

It has crashed because of this issue. I compiled it with GCC with debug symbols and identified the Problem with gdb. BTW: You can not know if every compiler with every settings does evaluate the disjunctions as you describe.

[hendu]

The C standard specifies that.

[REDDemon]

[typo] seems COGLES2 Material Renderer is included twice here:

https://github.com/kexplo/irrlicht_ogl- ... nderer.cpp


@porcus, if the index passed is causing problems then it is set to a wrong value (and hence the crash is caused by the range guard on std::vector).
you have to track wich code is setting it to wrong value or just create a small program that reproduce that.