Rebase MD5 module against upstream#1065
Conversation
Also, minor cleanups. Enforce unsigned chars everywhere. Update bin2str FWIW, I'm not sure what the masking was about as buf is an uint32_t? It never was in upstream's code. Will everything blow up? Will it magically fix the mysterious Clang failures? Noone knows!
Frenzie
left a comment
There was a problem hiding this comment.
Sure, looks functionally identical
| ---- @treturn string md5 sum in Lua string | ||
| function md5_proto:sum(luastr) | ||
| local buf = ffi.new("char[33]") | ||
| local buf = ffi.new("unsigned char[33]") |
There was a problem hiding this comment.
Doubtful that it would affect anything, I actually quite explicitly played around with this very change because I wondered if putting in an unsigned might help.
There was a problem hiding this comment.
It's unsigned on arm anyway ;).
(And mostly always explicitly an uint8_t in upstream's examples).
There was a problem hiding this comment.
Right, I'm just saying I did a few x86_64 experiments in Docker none of which made a difference on Clang-built LuaJIT. ;-) Regardless of cross-platform compatibility/best practices/etc., just to see if I could isolate something, anything at all.
There was a problem hiding this comment.
It's still especially confusing to me that locally it's a toss up or possibly as low as 1 in 4 that the problem occurs, while in the CI it's assured to go wrong.
There was a problem hiding this comment.
Yeah, I think I sometimes saw only 2 of the tests failing instead of the usual three during last night's runs. :?
There was a problem hiding this comment.
Oh, it does? Huh, I've always seen all three on CircleCI.
|
Hold up, this looks new? O_o |
|
Nope, that one has been happening sporadically since as far back as I can remember ;). (I can even reproduce that one locally :D). |
Regression since koreader#1065 Probably because of implicit type conversions to/from Lua and the fact that Lua numbers are double internally? Or something.
Regression since koreader#1065 Probably because of implicit type conversions to/from Lua and the fact that Lua numbers are double internally? Or something.
Regression since #1065 Probably because of implicit type conversions to/from Lua and the fact that Lua numbers are double internally? Or something.
(Same upstream as luxl, which is why I fell into this rabbit hole).
My vague hope that this might fix the mysterious Clang bot issues have been crushed.
On the other hand, it doesn't seem to break anything, so, eh.
This change is