Balazs Scheidler <[EMAIL PROTECTED]> writes:
> I just forgot to attach the patch previous time. Here it is.
Thanks. I've added most of it now (but I'll have to get it to compile
before checking it in). I have a few comments and questions.
I'm afraid that the flow control patches does not quite fix flow
control. Consider this scenario: We are receiving a lot of data, so
that our buffers fill up, and the other end runs out of window space.
The ssh channel is effectively put to sleep. Later, we manage to empty
our buffers, and we now want to send a WINDOW_ADJUST message to wake
up the channel. To do this, we need some sort of callback from the
write_buffer (or rather write_buffer_s_; there may be several buffers
sharing window space, e.g. stdout and stderr).
Some time ago, I added some code to the write_buffer object to have
the length instance variable track the current amount of buffered
data. This information is what has to be fed back into the
WINDOW_ADJUST logic. I want to figure out how to do this properly
before hacking the flow control. Also, I'm not sure using the
LSH_READY_REC for this is right; it was intended as a startup flag
only. If there's anything in your code that I have missed, please say
so.
You moved the CFMAKERAW call between two functions in client_pty.c.
Can you explain why that was needed? As far as I can tell, it should
work fine to do CFMAKERAW on the ios struct early, and then use it
later to actually set the tty attributes.
I added the IDEA glue right away (although I'm not entirely sure about
the political consequences of supporting IDEA).
About setting the pty permissions on bsd-style ptys, the SysV code
sets the permissions on the slave pty. Your code changes the
permissions on the master pty instead,
+ if (pty_grantpt_uid(master, user))
Is this intentional? Perhaps we should set the permissions on both
master and slave? In any case, my understanding is that changing the
permissions here does not make the bsd pty usage secure, as some other
user might keep the ptys open. But it should be better than nothing. I
also moved the permission stuff into a separate function used by both
SysV and BSD handling. And I disabled the openpty method, as I don't
know how that interacts with the pty permissions.
I added a compile time BASH_WORKAROUND option, enabled by default. The
only significant difference from your patch, I think, is that I leave
the session->err field as NULL in this case.
Best regards,
/Niels
