<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the patch!</div><div>LGTM, except for a few nits below.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16752378211329097630_BODY">From: Mike Pall <mike><br><br>Thanks to Jesper Lundgren.<br><br>(cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)<br><br>When `strftime()` returns empty result (for example, for "%p" on some<br>locales or when LuaJIT is built with musl's `strftime()` and format<br>string is invalid), the `os.date()` endless retries to format result</div></div></div></div></blockquote><div>Typo: s/endless/endlessly/</div><div>Typo: s/format result/format the result/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>and reallocates buffer for resulting string. This leads to OOM.</div></div></div></div></blockquote><div>Typo: s/buffer for resulting/the buffer for the resulting/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>This patch limits amount of retries by 4.</div></div></div></div></blockquote><div>Typo: s/amount/the amount/</div><div>Side note: Why is it exactly 4?</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>Sergey Kaplun:<br>* added the description and the test for the problem<br><br>Part of tarantool/tarantool#8069<br>---<br><br>PR: <a href="https://github.com/tarantool/tarantool/pull/8237" target="_blank">https://github.com/tarantool/tarantool/pull/8237</a><br>Issues:<br>* <a href="https://github.com/LuaJIT/LuaJIT/issues/463" target="_blank">https://github.com/LuaJIT/LuaJIT/issues/463</a><br>* <a href="https://github.com/tarantool/tarantool/issues/8069" target="_blank">https://github.com/tarantool/tarantool/issues/8069</a><br>Branch: <a href="https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci" target="_blank">https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci</a><br><br> src/lib_os.c | 4 ++--<br> .../lj-463-os-date-oom.test.lua | 19 +++++++++++++++++++<br> 2 files changed, 21 insertions(+), 2 deletions(-)<br> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua<br><br>diff --git a/src/lib_os.c b/src/lib_os.c<br>index 9e78d49a..ffbc3fdc 100644<br>--- a/src/lib_os.c<br>+++ b/src/lib_os.c<br>@@ -205,12 +205,12 @@ LJLIB_CF(os_date)<br> setboolfield(L, "isdst", stm->tm_isdst);<br> } else if (*s) {<br> SBuf *sb = &G(L)->tmpbuf;<br>- MSize sz = 0;<br>+ MSize sz = 0, retry = 4;<br> const char *q;<br> for (q = s; *q; q++)<br> sz += (*q == '%') ? 30 : 1; /* Overflow doesn't matter. */<br> setsbufL(sb, L);<br>- for (;;) {<br>+ while (retry--) { /* Limit growth for invalid format or empty result. */<br> char *buf = lj_buf_need(sb, sz);<br> size_t len = strftime(buf, sbufsz(sb), s, stm);<br> if (len) {<br>diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua<br>new file mode 100644<br>index 00000000..cce78b6e<br>--- /dev/null<br>+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua<br>@@ -0,0 +1,19 @@<br>+local tap = require('tap')<br>+<br>+-- See also <a href="https://github.com/LuaJIT/LuaJIT/issues/463" target="_blank">https://github.com/LuaJIT/LuaJIT/issues/463</a>.<br>+local test = tap.test('lj-463-os-date-oom')<br>+test:plan(1)<br>+<br>+-- The ru_RU.utf8 locale is chosen as one that will be set on a<br>+-- developer's PC with high possibility. It may be unavailable at</div></div></div></div></blockquote><div>Typo: s/with high possibility/with a high probability/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+-- CI, so don't check the status and result of function calls.<br>+-- If it's unavailable `os.setlocale()` does nothing.<br>+-- Before the patch, the call to `os.date('%p')` on non-standard<br>+-- locale may lead to OOM.<br>+<br>+os.setlocale('ru_RU.utf8')<br>+os.date('%p')<br>+<br>+test:ok(true, 'os.date() finished without OOM')<br>+<br>+os.exit(test:check() and 0 or 1)<br>--<br>2.34.1</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div><div> </div></div></blockquote></BODY></HTML>