tomaswolf commented on a change in pull request #214:
URL: https://github.com/apache/mina-sshd/pull/214#discussion_r840247098



##########
File path: 
sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java
##########
@@ -908,7 +908,7 @@ protected void closeShell(int exitValue, boolean 
closeImmediately) throws IOExce
 
         if (!isClosing()) {
             if (out != null) {
-                out.flush();

Review comment:
       `out.close()` works because this is indeed the last use of this stream. 
Looks fine to me...
   
   Unrelated to this change:
   
   But the code is a bit strange anyway. The intent is obviously to get all 
data from the command sent before the EOF is sent. But why not also flush or 
close `err`? What happens if the command writes something to the error stream? 
`err` gets closed only in the `close()` call in line 916, so that would flush 
it after the EOF (and the exit status) was sent. That's probably not OK.
   
   And anyway: stderr is traditionally "unbuffered", i.e., flushed on newlines. 
I don't see anything in ChannelOutputStream that would implement that for 
SSH_MSG_CHANNEL_EXTENDED_DATA. Maybe that's OK; the command would be 
responsible for flushing it frequently then if desired.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Reply via email to