Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/132#discussion_r161149994
--- Diff: src/terminal/terminal.c ---
@@ -255,52 +257,203 @@ void* guac_terminal_thread(void* data) {
}
-guac_terminal* guac_terminal_create(guac_client* client,
- const char* font_name, int font_size, int dpi,
- int width, int height, const char* color_scheme) {
+/**
+ * Compare a non-null-terminated string spanning token_start to token_end,
with
+ * a null-terminating string at literal.
+ */
+static int guac_terminal_color_scheme_compare_token(const char*
token_start,
+ const char* token_end, const char* literal) {
- int default_foreground;
- int default_background;
+ const int result = strncmp(literal, token_start, token_end -
token_start);
+ if (result != 0)
+ return result;
- /* Default to "gray-black" color scheme if no scheme provided */
- if (color_scheme == NULL || color_scheme[0] == '\0') {
- default_foreground = GUAC_TERMINAL_COLOR_GRAY;
- default_background = GUAC_TERMINAL_COLOR_BLACK;
- }
+ /* At this point, literal is same length or longer than
+ * | token_end - token_start |, so if the two are equal, literal should
+ * have its null-terminator at | token_end - token_start |. */
+ return (int) (unsigned char) literal[token_end - token_start];
+}
- /* Otherwise, parse color scheme */
- else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_GRAY_BLACK) == 0) {
- default_foreground = GUAC_TERMINAL_COLOR_GRAY;
- default_background = GUAC_TERMINAL_COLOR_BLACK;
- }
- else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_BLACK_WHITE) == 0) {
- default_foreground = GUAC_TERMINAL_COLOR_BLACK;
- default_background = GUAC_TERMINAL_COLOR_WHITE;
- }
- else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_GREEN_BLACK) == 0) {
- default_foreground = GUAC_TERMINAL_COLOR_DARK_GREEN;
- default_background = GUAC_TERMINAL_COLOR_BLACK;
- }
- else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_WHITE_BLACK) == 0) {
- default_foreground = GUAC_TERMINAL_COLOR_WHITE;
- default_background = GUAC_TERMINAL_COLOR_BLACK;
- }
+/**
+ * Parse a color-scheme configuration string, and return specified
+ * foreground/background colors and color palette.
+ *
+ * @param client
+ *
+ * @param color_scheme
+ * A semicolon-separated list of name-value pairs, i.e.
+ * "<name>: <value> [; <name>: <value> [; ...]]".
+ * For example, "color2: rgb:cc/33/22; background: color5".
+ *
+ * @param foreground
+ * (out) Parsed foreground color.
+ *
+ * @param background
+ * (out) Parsed background color.
+ *
+ * @param palette
+ * (out) Parsed color palette. Null is returned if color_scheme does
not
+ * require a custom palette. Otherwise, malloc() is used to allocate a
new
+ * palette, which must be freed manually by the calling code.
+ */
+static void guac_terminal_parse_color_scheme(guac_client* client,
+ const char* color_scheme, guac_terminal_color* foreground,
+ guac_terminal_color* background,
+ const guac_terminal_color (**palette)[256]) {
+
+ /* Set default gray-black color scheme and initial palette. */
+ *foreground = GUAC_TERMINAL_INITIAL_PALETTE[GUAC_TERMINAL_COLOR_GRAY];
+ *background = GUAC_TERMINAL_INITIAL_PALETTE[GUAC_TERMINAL_COLOR_BLACK];
+ *palette = NULL;
+
+ const guac_terminal_color (*ref_palette)[256] =
&GUAC_TERMINAL_INITIAL_PALETTE;
+ guac_terminal_color (*new_palette)[256] = NULL;
+
+ /* Pointer to the next token to parse. */
+ const char* next_token = color_scheme;
+
+ /* True if expecting the current token to be a value; false if
expecting a name. */
+ bool expect_value = false;
+
+ /* True if current token should be skipped. */
+ bool skip_token = false;
+
+ /* Target to place the parsed color value into. */
+ guac_terminal_color* value_target = NULL;
+
+ /* Terminator for the current token. */
+ char token_terminator = ';';
+
+ while (next_token) {
+ /* Use token_start and token_end to denote the current token. */
+ const char* token_start = next_token;
+ /* Strip leading spaces in current token. */
+ while (isspace(*token_start))
+ token_start++;
+
+ /* Find where this token ends */
+ const char* token_end = strpbrk(token_start, ":;");
+ if (token_end) {
+ next_token = token_end + 1;
+ token_terminator = *token_end;
+ }
+ else {
+ /* We've reached the end of the string. */
+ next_token = NULL;
+ token_end = token_start + strlen(token_start);
+ token_terminator = ';';
+ }
+ /* Strip trailing spaces in current token. */
+ while (token_end > token_start && isspace(*(token_end - 1)))
+ token_end--;
+
+ if (token_start == token_end) {
+ if (expect_value) {
+ guac_client_log(client, GUAC_LOG_WARNING,
+ "Empty color value at position %d.",
+ token_start - color_scheme);
+ return;
+ }
+ }
- /* If invalid, default to "gray-black" */
- else {
- guac_client_log(client, GUAC_LOG_WARNING,
- "Invalid color scheme: \"%s\". Defaulting to
\"gray-black\".",
- color_scheme);
- default_foreground = GUAC_TERMINAL_COLOR_GRAY;
- default_background = GUAC_TERMINAL_COLOR_BLACK;
+ else if (skip_token) {
+ expect_value = false;
+ skip_token = false;
+ }
+
+ else if (expect_value) {
+ expect_value = false;
+
+ /* Parse color<n> value. */
+ int index = -1;
+ if (sscanf(token_start, GUAC_TERMINAL_SCHEME_NUMBERED "%d",
&index) &&
+ index >= 0 && index <= 255) {
+ *value_target = (*ref_palette)[index];
+ }
+
+ /* Parse X11 value. */
+ else if (!guac_terminal_xparsecolor(token_start,
value_target)) {
+ if (!guac_terminal_color_scheme_compare_token(token_start,
+ token_end,
"rgb")) {
+ /* This value was a "rgb:R/G/B" value, so the next
token
+ * will be the "R/G/B" part. Skip processing that
part. */
+ expect_value = true;
+ skip_token = true;
+ }
+ }
+
+ else {
+ guac_client_log(client, GUAC_LOG_WARNING,
+ "Invalid color value: \"%.*s\".",
+ token_end - token_start, token_start);
+ return;
+ }
+ }
+
+ /* Expecting a name. */
+ else if (!guac_terminal_color_scheme_compare_token(
+ token_start, token_end, GUAC_TERMINAL_SCHEME_FOREGROUND)) {
+ value_target = foreground;
+ expect_value = true;
+ }
+
+ else if (!guac_terminal_color_scheme_compare_token(
+ token_start, token_end, GUAC_TERMINAL_SCHEME_BACKGROUND)) {
+ value_target = background;
+ expect_value = true;
+ }
+
+ else {
+ /* Parse color<n> value. */
+ int index = -1;
+ if (sscanf(token_start, GUAC_TERMINAL_SCHEME_NUMBERED "%d",
&index) &&
+ index >= 0 && index <= 255) {
+
+ /* Allocate a new palette now that we are mutating it. */
+ if (!new_palette) {
+ const size_t size = sizeof(guac_terminal_color[256]);
+ new_palette = (guac_terminal_color(*)[256])
malloc(size);
--- End diff --
Why conditionally allocate space, copy, etc.? It seems like the caller
should provide initialized space (presumably a copy of
`GUAC_TERMINAL_INITIAL_PALETTE`), with the scheme parser mutating that provided
space as specified, no?
---
