Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/11#discussion_r71967043
--- Diff: src/common-ssh/guac_ssh.c ---
@@ -275,11 +275,43 @@ static void guac_common_ssh_kbd_callback(const char
*name, int name_len,
}
/**
+ * A handler for SSH client errors which logs the error and can optionally
+ * abort the given guac_client with the given guac_protocol_status.
+ *
+ * @param client
+ * Guacamole client instance to use when logging or aborting.
+ *
+ * @param status
+ * Protocol status to return if aborting the connection. This is only
+ * used if abort_on_error is set to true.
+ *
+ * @param abort_on_error
+ * Whether to abort the connection as well as log the error.
+ *
+ * @param format
+ * Format string for log messages. This is also used when aborting
+ * the connection.
+ *
+ */
+static void guac_common_ssh_client_error(guac_client* client,
+ guac_protocol_status status, int abort_on_error, const char*
format, ...) {
+ va_list args;
+ va_start(args, format);
+ if(abort_on_error == 1) {
--- End diff --
Two things here:
1. Direct comparisons against `1` are a little bit funky in C. Normally,
you use non-zero to represent true, as this is what C uses internally for `if`
tests, etc. Things will work more as expected with:
```
if (abort_on_error) {
...
}
```
2. `if` is not a function. There should be a space between the `if` and the
opening paren.
Beyond that, I think this is OK. The function seems a bit crowded to me due
to the lack of blank lines / "code paragraphs", but that's not a deal-breaker.
I'd like you to consider omitting the braces where unnecessary (as that's our
preference - see the [style
guidelines](http://guacamole.incubator.apache.org/guac-style/)), but as we say
in that doc, these aren't hard rules - use your own best judgement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
