On Mon, 2 Dec 2024 16:31:17 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Actually, IIANM, shouldn't it be + 5 here and + 3 below to account for the >> null terminator? >> snprintf says it will write up to size - 1 characters. > > You are right about + 3 instead of + 2. Furthermore, now that you mention it, > I'm not sure why there's a `strlen(format)` in that size computation. The > original change was introduced in > https://hg.openjdk.org/jdk7/jdk7/jdk/rev/b5d37597c815 and at that time it was > using `sprintf` (and not `snprintf`) and the `sprintf` as per its > documentation, considers size to be `INT_MAX + 1`, so `size` wasn't being > passed to it and only used for `malloc()`. Even then, I don't understand why > `strlen(format)` (or even + 2) was considered for the size. > > I have updated this part of the PR, but I'm going to look at this size > computation bit more tomorrow with a fresh mind. oh - yes - the `strlen(format)` is strange - maybe it allowed to accout for the various ": ", and the rest would be > 1 which would allow for the null terminator? But then + 2 wouldn't have been needed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22484#discussion_r1866224359
