exit in utf8ncmp if over limit before reading

[bitonic]

this makes the function safe on strings that are not null-terminated.

[sheredom]

Hey there - thanks for the contribution! Could you please add a test case that would have failed without this change? Just so that it never regresses again in the future.

[bitonic]

@sheredom it is not trivial to set up a reliable test for this, and the failure mode would be a segfault or similar.

however, if you're running valgrind or something like that on CI in the tests it is easy to make valgrind fail with the previous function, so let me know if this is the case.

[sheredom]

Hm interesting! Could you just post a code snippet on this pull request showing the failure then? That way when I get round to adding clang sanitizers or valgrind I should have a test case ready to use 😄

[bitonic]

@sheredom (not tested, but should convey the idea):

  unsigned char *chrs1 = (unsigned char *) malloc(1);
  chrs1[0] = 'a';
  unsigned char *chrs2 = (unsigned char *) malloc(1);
  chrs2[0] = 'a';
  utf8ncmp((void *) chrs1, (void *) chrs2, 1);

before this PR, utf8ncmp will try to read beyond the first char, and in the case above that'd be unsafe.

[sheredom]

@bitonic thanks for the fix - and for the example! Always happy to have new contributors 😄

[bitonic]

thanks for the very useful code! I'm also thinking of changing the if branches in many of the functions to go through the single byte case first since it should be a lot more common -- right now it goes through the 4-byte version first. I first wanted to check if you had some benchmarks first though 🙃.

…

On Sun, 20 Jan 2019 at 10:24, Neil Henning ***@***.***> wrote: @bitonic <https://github.com/bitonic> thanks for the fix - and for the example! Always happy to have new contributors 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#57 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh8OoiA3poXHww8y7i64c4ZrtO6bGNlks5vFDW3gaJpZM4aJifQ> .

[sheredom]

I do not have any benchmarks for that - but if you find it is faster to switch the branches around feel free to submit that PR!