search

koreader / crengine

This is the KOReader CREngine fork. It cross-pollinates with the official CoolReader repository at https://github.com/buggins/coolreader, in case you were looking for that one.
70 stars 45 forks source link

lvstring: fix size handling #538

Closed benoit-pierre closed 10 months ago

benoit-pierre commented 10 months ago

The chunk size is the allocated size minus one (the terminating \0). Fix methods that don't have the same definition.

This change is Reviewable

poire-z commented 10 months ago

Looks like it is the case indeed. This is all confirmed by the fact there is n+1 in all the calls to malloc & realloc : sizeof(char8/32) * (n+1), right? So, we were just always allocating one slot too much, that ended up just being not used ? Any other method that was using size right ?

benoit-pierre commented 10 months ago

Looks like it is the case indeed. This is all confirmed by the fact there is n+1 in all the calls to malloc & realloc : sizeof(char8/32) * (n+1), right? So, we were just always allocating one slot too much, that ended up just being not used ?

No, the +1 allocation is right (for the extra terminating \0), again size is the usable size.

The problem is setting size to n+1, which means a following call to append('c') for example would overflow the buffer by one character.

Any other method that was using size right ?

Yes, reserve.

poire-z commented 10 months ago

No, the +1 allocation is right (for the extra terminating \0), again size is the usable size.

I meant it the other way around: It's because size is indeed the usable size that the code was already using everywhere malloc & realloc : sizeof(char8/32) * (n+1). Or I still don't get it ? :/

benoit-pierre commented 10 months ago

No, the +1 allocation is right (for the extra terminating \0), again size is the usable size.

I meant it the other way around: It's because size is indeed the usable size that the code was already using everywhere malloc & realloc : sizeof(char8/32) * (n+1).

Yes, and setting size = n, and checking for not enough capacity with pchunk->size < len.

poire-z commented 10 months ago

Same dilemna as in https://github.com/koreader/crengine/pull/543#issuecomment-1781920417 in here. Waiting for a common decision :)