support larger than 1k client responses (bigger winfmts, etc) #43
Conversation
smemsh
force-pushed
the
large-cmd-responses
branch
from
a58d08f to
b795f08
Compare
This patch allows those of us with larger numbers of windows and use "windows" format strings to print stuff to wrapper scripts, to continue to use sdorfehs when switching from ratpoison. Before this patch, client would read only a single up-to-1k bytes buffer to get the entire response, resulting in truncation of the message (windows list, etc) if it was longer than 1022 bytes. Ultimately this was a consequence of retooling to a different IPC mechanism that happened in a426e8b, the new code uses a single read of 1024 bytes to get the response. First thought was to just make the response buffer larger, but who knows what uses people have? Can we predict the right size limit? This way it can handle any sized response. Server is still the one controlling size of result buffer, and fixed 1k is still used for the "request" buffer sent to the server process. Second thought was, since response already has "successfulness" as the first byte of response (and we have to process it), we could declare an "initial response" packet that would include some bytes that specified the response buffer size, and then client would always make a second second request, this time with an exact size it knew in advance. This may have been a fine approach, but results in an extra round trip for the common case of a single buffer. Thought a loop would be better... So, instead we treat the first buffer specially, by continuing to strip the successfulness response, but then also now arranging for subsequent reads to be O_NONBLOCK. Presence of the first buffer means that necessarily the server has prepared the full response, so we can keep going until a blocked or closed connection to get the rest of it. In testing, at response conclusion, sometimes the socket connection was closed, and sometimes the end of the response would just block forever waiting for another read. Was not able to tell what differentiates these end-states, but also found that treating them identically (as "command finished" signals) always seems to result in the right behavior, so it might be ok not to know why...
smemsh
force-pushed
the
large-cmd-responses
branch
from
b795f08 to
3c1e6b0
Compare
|
Too bad about all the I debated removing these prints after the code was working, but it took some time to develop the right points where those debug prints should be located, I had an occasional off-by-one that was difficult to track down. A new macro like For now, thought it best to ask for comment on what should be done with these prints... |
I added a different macro This will not print anything if Let me know if it's suitable, maybe you won't like those prints even in your debug builds. |
smemsh
force-pushed
the
large-cmd-responses
branch
2 times, most recently
from
b98e8eb to
38eb5f0
Compare
|
now I further hid those stderr macros under |
The client cannot use PRINT_DEBUG because it sends to stdout, which we cannot contaminate. We can use warn(), but don't want that to pollute the non-DEBUG experience, and we don't want to pollute -DDEBUG, so we add it under a new -DSENDCMD_DEBUG following the example of INPUT_DEBUG. However, to avoid ifdefs everywhere, we'll prefer to do that in a macro. So we just add another macro that uses stderr. It's a bit more compact than the one that uses stdout, this one uses __VA_ARGS__ as a straight text macro, this should compile on BSD and Linux (only latter tested). We could not use a modification of PRINT_DEBUG because it would have needed fprintf(file, ...) and the way the macro was written it would require passing ((args like this)) because it had a naked "printf args" and relied on getting first level "()" stripped in the macro expansion. This was a bit strange. It does mean there's now inconsistency between PRINT_DEBUG and WARNX_DEBUG usage (former needs '(())', latter '()') but the latter macro is only used by one function anyways and only appropriate for the client so usage will probably never expand.
smemsh
force-pushed
the
large-cmd-responses
branch
from
38eb5f0 to
cb343a2
Compare
Previously the buffer size could not exceed 1k in the response from server, or it would be truncated. This included commands like, for example:
would easily get over 1024 chars (actually 1022 due to \n and \0) on my system. The new code can handle any size response.
Fixes #39