mike-jumper commented on a change in pull request #327:
URL: https://github.com/apache/guacamole-server/pull/327#discussion_r569160096
##########
File path: src/protocols/vnc/vnc.c
##########
@@ -385,6 +385,17 @@ void* guac_vnc_client_thread(void* data) {
}
#endif
+ /* Disable remote console (Server input) */
+ if (settings->disable_server_input) {
+ rfbSetServerInputMsg msg;
+ msg.type = rfbSetServerInput;
+ msg.status = 1;
+ msg.pad = 0;
+
+ rfbBool success = WriteToRFBServer(rfb_client, (char*)&msg,
sz_rfbSetServerInputMsg);
+ guac_client_log(client, GUAC_LOG_DEBUG, "%s send request to disable
server input", success ? "Successfully" : "Failed to");
Review comment:
This reads well in the case of failure ("Failed to send request ..."),
but the grammar gets wonky if things are sent successfully ("Successfully send
request ...").
Perhaps it would be better to simply have two separate `guac_client_log()`
calls? That would also allow for the failure case to be logged at a higher log
level like `GUAC_LOG_WARNING` without getting too cluttered.
##########
File path: src/protocols/vnc/vnc.c
##########
@@ -385,6 +385,17 @@ void* guac_vnc_client_thread(void* data) {
}
#endif
+ /* Disable remote console (Server input) */
+ if (settings->disable_server_input) {
+ rfbSetServerInputMsg msg;
+ msg.type = rfbSetServerInput;
+ msg.status = 1;
+ msg.pad = 0;
Review comment:
FYI - I'm sure this is fine as-is (all structure members are explicitly
initialized here), but you can do this quickly and cleanly in one step with:
```c
rfbSetServerInputMsg msg = {
.type = rfbSetServerInput,
.status = 1
};
```
with `pad` implicitly zeroed.
I'm OK with things as you have them, but just wanted to point the above out.
We have additional syntax at our disposal here thanks to the codebase being
written for C99.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
