On Mon, 2 Dec 2024 16:53:32 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> 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. I'd keep the `strlen(format)` and remove the fixed `+2` part; using `strlen(format)` will allocate a bit more than needed, but will free us from the need to manually calculate the exact number of extra bytes required. Other than that, LGTM. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22484#discussion_r1925662409
