PopulateBidiChain in some situations leaves SBBidiType portion of memory uninitialized.

[RafalWawrzyniak-TomTom]

I checked with valgrind that on every run it reports "==2247== Warning: unimplemented fcntl command: 1033 ==2247== Thread 32: ==2247== Conditional jump or move depends on uninitialised value(s) ==2247== at 0x14B4107: SBAlgorithmCreateParagraph" I tried to narrow down the part of memory which is uninitialized according to valgrind and it came out that if I will add in CreateParagraphContext function just after BidiChainInitialize additional code

for (int i = 0;i < length;++i) fixedTypes[i] = SBBidiTypeNil;

the valgrind is not reporting any problem.

I followed PopulateBidiChain code and it looks that in some cases it can omit initialization of some links in the chain. I do not understand sheenbidi well enough to say if this mean we have a data error in such case. Anyway application is not crashing because of it, the only visible problem so far is valgrind report.

[mta452]

This can be safely ignored since uninitalized links are never read anyway. Basically it mimicks a linked list but with a pre allocated array. This helps overcome the malloc overhead of nodes.

The chain is iterated from the links indexes array, so it will always access a valid link.

[DominiquePelle-TomTom]

@mta452 wrote:

This can be safely ignored since uninitalized links are never read anyway.

That is still annoying, even if it presumably does not have bad consequences in this case.

One would expect to have clean report from valgrind, otherwise it becomes nearly impossible to know what's fine and what is done when checking software that integrates many 3rd party libs.

To be pedantic, a condition that depends on uninitialized memory is undefined behavior, and UB is always a bug (which may or may not have consequences in some cases, but that's unpredictable on all toolchains/platforms).

[InfoTeddy]

for (int i = 0;i < length;++i) fixedTypes[i] = SBBidiTypeNil;

This is incorrect, because the array is actually of length length + 2. So it should be:

for (SBUInteger i = 0; i < length + 2; ++i) {
    fixedTypes[i] = SBBidiTypeNil;
}

To be pedantic, a condition that depends on uninitialized memory is undefined behavior, and UB is always a bug (which may or may not have consequences in some cases, but that's unpredictable on all toolchains/platforms).

I very much agree. This affects us too (us being the VVVVVV developers) as we are looking to integrate SheenBidi (see TerryCavanagh/VVVVVV#1093) but this error keeps coming up in Valgrind.

Can we send a pull request to fix this?