Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/guacamole-server/pull/132#discussion_r189474807
  
    --- Diff: src/terminal/terminal.c ---
    @@ -255,53 +260,252 @@ 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,
    -        const int backspace) {
    +/**
    + * Compare a non-null-terminated string to a null-terminated literal, in 
the
    + * same manner as strcmp().
    + *
    + * @param str_start
    + *     Start of the non-null-terminated string.
    + *
    + * @param str_end
    + *     End of the non-null-terminated string, after the last character.
    + *
    + * @param literal
    + *     The null-terminated literal to compare against.
    + *
    + * @return
    + *     Zero if the two strings are equal and non-zero otherwise.
    + */
    +static int guac_terminal_color_scheme_compare_token(const char* str_start,
    +        const char* str_end, const char* literal) {
     
    -    int default_foreground;
    -    int default_background;
    +    const int result = strncmp(literal, str_start, str_end - str_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
    +     * | str_end - str_start |, so if the two are equal, literal should
    +     * have its null-terminator at | str_end - str_start |. */
    +    return (int) (unsigned char) literal[str_end - str_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;
    +/**
    + * Strip the leading and trailing spaces of a bounded string.
    + *
    + * @param[in,out] str_start
    + *     Address of a pointer to the start of the string. On return, the 
pointer
    + *     is advanced to after any leading spaces.
    + *
    + * @param[in,out] str_end
    + *     Address of a pointer to the end of the string, after the last 
character.
    + *     On return, the pointer is moved back to before any trailing spaces.
    + */
    +static void guac_terminal_color_scheme_strip_spaces(const char** str_start,
    +        const char** str_end) {
    +
    +    /* Strip leading spaces. */
    +    while (*str_start < *str_end && isspace(**str_start))
    +        (*str_start)++;
    +
    +    /* Strip trailing spaces. */
    +    while (*str_end > *str_start && isspace(*(*str_end - 1)))
    +        (*str_end)--;
    +}
    +
    +/**
    + * Parse the name part of the name-value pair within the color-scheme
    + * configuration.
    + *
    + * @param client
    + *
    + * @param name_start
    + *     Start of the name string.
    + *
    + * @param name_end
    + *     End of the name string, after the last character.
    + *
    + * @param foreground
    + *     Pointer to the foreground color.
    + *
    + * @param background
    + *     Pointer to the background color.
    + *
    + * @param palette
    + *     Pointer to the palette array.
    + *
    + * @param[out] target
    + *     On return, pointer to the color struct that corresponds to the name.
    + *
    + * @return
    + *     Zero if successful or non-zero otherwise.
    + */
    +static int guac_terminal_parse_color_scheme_name(guac_client* client,
    +        const char* name_start, const char* name_end,
    +        guac_terminal_color* foreground, guac_terminal_color* background,
    +        guac_terminal_color (*palette)[256],
    +        guac_terminal_color** target) {
    +
    +    guac_terminal_color_scheme_strip_spaces(&name_start, &name_end);
    +
    +    if (!guac_terminal_color_scheme_compare_token(
    +            name_start, name_end, GUAC_TERMINAL_SCHEME_FOREGROUND)) {
    +        *target = foreground;
    +        return 0;
         }
    -    else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_BLACK_WHITE) == 0) {
    -        default_foreground = GUAC_TERMINAL_COLOR_BLACK;
    -        default_background = GUAC_TERMINAL_COLOR_WHITE;
    +
    +    if (!guac_terminal_color_scheme_compare_token(
    +            name_start, name_end, GUAC_TERMINAL_SCHEME_BACKGROUND)) {
    +        *target = background;
    +        return 0;
         }
    -    else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_GREEN_BLACK) == 0) {
    -        default_foreground = GUAC_TERMINAL_COLOR_DARK_GREEN;
    -        default_background = GUAC_TERMINAL_COLOR_BLACK;
    +
    +    /* Parse color<n> value. */
    +    int index = -1;
    +    if (sscanf(name_start, GUAC_TERMINAL_SCHEME_NUMBERED "%d", &index) &&
    +            index >= 0 && index <= 255) {
    +        *target = &(*palette)[index];
    +        return 0;
         }
    -    else if (strcmp(color_scheme, GUAC_TERMINAL_SCHEME_WHITE_BLACK) == 0) {
    -        default_foreground = GUAC_TERMINAL_COLOR_WHITE;
    -        default_background = GUAC_TERMINAL_COLOR_BLACK;
    +
    +    guac_client_log(client, GUAC_LOG_WARNING,
    +                    "Unknown color name: \"%.*s\".",
    +                    name_end - name_start, name_start);
    +    return 1;
    +}
    +
    +/**
    + * Parse the value part of the name-value pair within the color-scheme
    + * configuration.
    + *
    + * @param client
    + *
    + * @param value_start
    + *     Start of the value string.
    + *
    + * @param value_end
    + *     End of the value string, after the last character.
    + *
    + * @param palette
    + *     The current color palette.
    + *
    + * @param[out] target
    + *     On return, the parsed color.
    + *
    + * @return
    + *     Zero if successful or non-zero otherwise.
    + */
    +static int guac_terminal_parse_color_scheme_value(guac_client* client,
    +        const char* value_start, const char* value_end,
    +        const guac_terminal_color (*palette)[256],
    +        guac_terminal_color* target) {
    +
    +    guac_terminal_color_scheme_strip_spaces(&value_start, &value_end);
    +
    +    /* Parse color<n> value. */
    +    int index = -1;
    +    if (sscanf(value_start, GUAC_TERMINAL_SCHEME_NUMBERED "%d", &index) &&
    +            index >= 0 && index <= 255) {
    +        *target = (*palette)[index];
    +        return 0;
         }
     
    -    /* 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;
    +    /* Parse X11 value. */
    +    if (!guac_terminal_xparsecolor(value_start, target))
    +        return 0;
    +
    +    guac_client_log(client, GUAC_LOG_WARNING,
    +                    "Invalid color value: \"%.*s\".",
    +                    value_end - value_start, value_start);
    +    return 1;
    +}
    +
    +/**
    + * Parse a color-scheme configuration string, and return specified
    + * foreground/background colors and color palette.
    + *
    + * @param client
    --- End diff --
    
    Please document this parameter as well.


---

Reply via email to