On Sun, 7 Apr 2024 at 22:05, Jelte Fennema-Nio <postg...@jeltef.nl> wrote: > > On Sun, 7 Apr 2024 at 03:39, Andres Freund <and...@anarazel.de> wrote: > > Changing the global vars to size_t seems mildly bogus to me. All it's > > achieving is to use slightly more memory. It also just seems unrelated to > > the > > change. > > I took a closer look at this. I agree that changing PqSendBufferSize > to size_t is unnecessary: given the locations that it is used I see no > risk of overflow anywhere. Changing the type of PqSendPointer and > PqSendStart is needed though, because (as described by Heiki and David > upthread) the argument type of internal_flush_buffer is size_t*. So if > you actually pass int* there, and the sizes are not the same then you > will start writing out of bounds. And because internal_flush_buffer is > introduced in this patch, it is related to this change. > > This is what David just committed too. > > However, the "required" var actually should be of size_t to avoid > overflow if len is larger than int even without this change. So > attached is a tiny patch that does that.
Looking at the code in socket_putmessage_noblock(), I don't understand why it's ok for PqSendBufferSize to be int but "required" must be size_t. There's a line that does "PqSendBufferSize = required;". It kinda looks like they both should be size_t. Am I missing something that you've thought about? David