On Tue, 10 Dec 2024 09:52:12 GMT, Kevin Walls <kev...@openjdk.org> wrote:

> Looks good, thanks, a couple of other notes here in case it helps anyone.
> 
> AttachOperation::write_reply(ReplyWriter * writer, jint result, const char* 
> message, int message_len) is this only called by the write_reply method that 
> passes message_len as strlen(stream) ? The new check for len<0 might not 
> really be useful but does no harm.

There are couple calls from `AttachOperation::read_request` that passes `const 
char*` (`message_len` is -1 as the default value).
I.e we need 2 versions of write_reply - one for `bufferedStream` and another 
for `const char*`.

> The existing "char msg[32]" is a buffer used to write the numeric jint 
> result, not the message as the name might suggest.

This is old code (similar to deleted from `PosixAttachOperation::complete`).
Do you want to rename `msg` to `buf`?

> I'm sure we all want to find time for the future conversation about actually 
> streaming the output rather than passing a single buffer around. 8-)

I'm open for it :)

-------------

PR Comment: https://git.openjdk.org/jdk/pull/22223#issuecomment-2532670222

Reply via email to