From b4b381e94727776ca3ba63f240f5b5f87fc7a087 Mon Sep 17 00:00:00 2001 From: Pierre Habouzit Date: Sun, 22 Jun 2008 17:45:45 +0200 Subject: [PATCH] Finish markup parsing rewrite to avoid mallocs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For that matter, use elements as a filter for elements we care about, and let the hook implement whatever it needs without duplicating everything. The resulting algorithm is still O(n²) where n is the number of filtered elements (3 at most right now), which isn't bad if we don't need to get too many elements, but at least it's not quadratic in the number of attributes anymore. Speedup improvements could be done using gperf btw. Signed-off-by: Pierre Habouzit --- client.c | 24 ++++++++++---- common/draw.c | 83 ++++++++++++++++++++++++++++------------------ common/markup.c | 51 +++------------------------- common/markup.h | 18 ++++++---- widgets/taglist.c | 20 ++++++++--- widgets/tasklist.c | 49 +++++++++++++++++++-------- 6 files changed, 137 insertions(+), 108 deletions(-) diff --git a/client.c b/client.c index 6f1e0e70..09a70933 100644 --- a/client.c +++ b/client.c @@ -759,6 +759,17 @@ client_updatesizehints(client_t *c) return size; } +/** Markup parsing hook. + * For now only substitute + */ +static void +client_markup_on_elem(markup_parser_data_t *p, const char *elem, + const char **names, const char **values) +{ + assert (!strcmp(elem, "title")); + buffer_add_xmlescaped(&p->text, p->priv); +} + /** Parse a markup string which contains special markup sequence relative to a * client, i.e. its title, etc. * \param c The client concerned by the markup string. @@ -768,17 +779,18 @@ client_updatesizehints(client_t *c) char * client_markup_parse(client_t *c, const char *str, ssize_t len) { - const char *elements[] = { "title", NULL }; - const char *elements_sub[] = { c->name , NULL }; - markup_parser_data_t p; + static char const * const elements[] = { "title", NULL }; + markup_parser_data_t p = { + .elements = elements, + .priv = c->name, + .on_element = client_markup_on_elem, + }; char *ret; - markup_parser_data_init(&p, elements, elements_sub, countof(elements)); + markup_parser_data_init(&p); if(markup_parse(&p, str, len)) - { ret = buffer_detach(&p.text); - } else ret = a_strdup(str); diff --git a/common/draw.c b/common/draw.c index 7a18aeff..c7ee6508 100644 --- a/common/draw.c +++ b/common/draw.c @@ -216,15 +216,62 @@ typedef struct } shadow; } draw_parser_data_t; +static void +draw_markup_on_event(markup_parser_data_t *p, const char *elem, + const char **names, const char **values) +{ + draw_parser_data_t *data = p->priv; + + /* hack: markup.c validates tags so we can avoid strcmps here */ + switch (*elem) { + case 'b': /* bg */ + for (; *names; names++, values++) + { + if(!a_strcmp(*names, "color")) + data->has_bg_color = xcolor_new(data->connection, data->phys_screen, + *values, &data->bg_color); + else if(!a_strcmp(*names, "image")) + data->bg_image = draw_image_new(*values); + } + break; + + case 't': /* text */ + for (; *names; names++, values++) + { + if(!a_strcmp(*names, "align")) + data->align = draw_align_get_from_str(*values); + else if(!a_strcmp(*names, "shadow")) + xcolor_new(data->connection, data->phys_screen, *values, + &data->shadow.color); + else if(!a_strcmp(*names, "shadow_offset")) + data->shadow.offset = atoi(*values); + } + break; + + case 'm': /* margin */ + for (; *names; names++, values++) + { + if(!a_strcmp(*names, "left")) + data->margin.left = atoi(*values); + else if(!a_strcmp(*names, "right")) + data->margin.right = atoi(*values); + } + break; + } +} + static bool draw_text_markup_expand(draw_parser_data_t *data, const char *str, ssize_t slen) { - const char *elements[] = { "bg", "text", "margin", NULL }; - markup_parser_data_t p; - int i; + static char const * const elements[] = { "bg", "text", "margin", NULL }; + markup_parser_data_t p = { + .elements = elements, + .priv = data, + .on_element = &draw_markup_on_event, + }; - markup_parser_data_init(&p, elements, NULL, countof(elements)); + markup_parser_data_init(&p); if(!markup_parse(&p, str, slen)) { @@ -232,34 +279,6 @@ draw_text_markup_expand(draw_parser_data_t *data, return false; } - /* bg */ - if(p.attribute_names[0]) - for(i = 0; p.attribute_names[0][i]; i++) - if(!a_strcmp(p.attribute_names[0][i], "color")) - data->has_bg_color = xcolor_new(data->connection, data->phys_screen, - p.attribute_values[0][i], &data->bg_color); - else if(!a_strcmp(p.attribute_names[0][i], "image")) - data->bg_image = draw_image_new(p.attribute_values[0][i]); - - /* text */ - if(p.attribute_names[1]) - for(i = 0; p.attribute_names[1][i]; i++) - if(!a_strcmp(p.attribute_names[1][i], "align")) - data->align = draw_align_get_from_str(p.attribute_values[1][i]); - else if(!a_strcmp(p.attribute_names[1][i], "shadow")) - xcolor_new(data->connection, data->phys_screen, - p.attribute_values[1][i], &data->shadow.color); - else if(!a_strcmp(p.attribute_names[1][i], "shadow_offset")) - data->shadow.offset = atoi(p.attribute_values[1][i]); - - /* margin */ - if(p.attribute_names[2]) - for(i = 0; p.attribute_names[2][i]; i++) - if(!a_strcmp(p.attribute_names[2][i], "left")) - data->margin.left = atoi(p.attribute_values[2][i]); - else if(!a_strcmp(p.attribute_names[2][i], "right")) - data->margin.right = atoi(p.attribute_values[2][i]); - /* stole text */ data->text = p.text; buffer_init(&p.text); diff --git a/common/markup.c b/common/markup.c index 070ab3b6..9de51f68 100644 --- a/common/markup.c +++ b/common/markup.c @@ -47,26 +47,12 @@ markup_parse_start_element(GMarkupParseContext *context __attribute__ ((unused)) GError **error __attribute__ ((unused))) { markup_parser_data_t *p = (markup_parser_data_t *) user_data; - int i, j; + int i; for(i = 0; p->elements[i]; i++) - if(!a_strcmp(element_name, p->elements[i])) - { - for(j = 0; attribute_names[j]; j++); - p->attribute_names[i] = p_new(char *, ++j); - p->attribute_values[i] = p_new(char *, j); - - for(j = 0; attribute_names[j]; j++) - { - p->attribute_names[i][j] = a_strdup(attribute_names[j]); - p->attribute_values[i][j] = a_strdup(attribute_values[j]); - } - - if(p->elements_sub && p->elements_sub[i]) - { - buffer_add_xmlescaped(&p->text, p->elements_sub[i]); - } - + if(!a_strcmp(element_name, p->elements[i])) { + (*p->on_element)(p, element_name, attribute_names, + attribute_values); return; } @@ -134,20 +120,10 @@ markup_parse_text(GMarkupParseContext *context __attribute__ ((unused)), /** Create a markup_parser_data_t structure with elements list. * \param a pointer to an allocated markup_parser_data_t which must be wiped * with markup_parser_data_wipe() - * \param elements an array of elements to look for, NULL terminated - * \param elements_sub an optional array of values to substitude to elements, NULL - * terminated, or NULL if not needed - * \param nelem number of elements in the array (without NULL) */ -void -markup_parser_data_init(markup_parser_data_t *p, const char **elements, - const char **elements_sub, ssize_t nelem) +void markup_parser_data_init(markup_parser_data_t *p) { buffer_init(&p->text); - p->elements = elements; - p->elements_sub = elements_sub; - p->attribute_names = p_new(char **, nelem); - p->attribute_values = p_new(char **, nelem); } /** Wipe a markup_parser_data_t initialized with markup_parser_data_init. @@ -156,23 +132,6 @@ markup_parser_data_init(markup_parser_data_t *p, const char **elements, void markup_parser_data_wipe(markup_parser_data_t *p) { - int i, j; - - for(i = 0; p->elements[i]; i++) - if(p->attribute_names[i]) - { - for(j = 0; p->attribute_names[i][j]; j++) - { - p_delete(&p->attribute_names[i][j]); - p_delete(&p->attribute_values[i][j]); - } - p_delete(&p->attribute_names[i]); - p_delete(&p->attribute_values[i]); - } - - p_delete(&p->attribute_names); - p_delete(&p->attribute_values); - buffer_wipe(&p->text); } diff --git a/common/markup.h b/common/markup.h index 54a47b69..e14d30c9 100644 --- a/common/markup.h +++ b/common/markup.h @@ -24,16 +24,20 @@ #include "common/buffer.h" -typedef struct +typedef struct markup_parser_data_t markup_parser_data_t; + +typedef void (markup_on_elem_f)(markup_parser_data_t *, const char *, + const char **, const char **); + +struct markup_parser_data_t { buffer_t text; - const char **elements; - const char **elements_sub; - char ***attribute_names; - char ***attribute_values; -} markup_parser_data_t; + const char * const *elements; + markup_on_elem_f *on_element; + void *priv; +}; -void markup_parser_data_init(markup_parser_data_t *, const char **, const char **, ssize_t); +void markup_parser_data_init(markup_parser_data_t *); void markup_parser_data_wipe(markup_parser_data_t *); bool markup_parse(markup_parser_data_t *data, const char *, ssize_t); diff --git a/widgets/taglist.c b/widgets/taglist.c index 75af0e75..64df4e72 100644 --- a/widgets/taglist.c +++ b/widgets/taglist.c @@ -54,15 +54,27 @@ typedef struct } taglist_data_t; +static void +tag_markup_on_elem(markup_parser_data_t *p, const char *elem, + const char **names, const char **values) +{ + assert (!strcmp(elem, "title")); + buffer_add_xmlescaped(&p->text, p->priv); +} + + static char * tag_markup_parse(tag_t *t, const char *str, ssize_t len) { - const char *elements[] = { "title", NULL }; - const char *elements_sub[] = { t->name , NULL }; - markup_parser_data_t p; + static char const * const elements[] = { "title", NULL }; + markup_parser_data_t p = { + .elements = elements, + .on_element = &tag_markup_on_elem, + .priv = t->name, + }; char *ret; - markup_parser_data_init(&p, elements, elements_sub, countof(elements)); + markup_parser_data_init(&p); if(markup_parse(&p, str, len)) ret = buffer_detach(&p.text); diff --git a/widgets/tasklist.c b/widgets/tasklist.c index 3a27c888..3b2873c8 100644 --- a/widgets/tasklist.c +++ b/widgets/tasklist.c @@ -63,6 +63,31 @@ tasklist_isvisible(client_t *c, int screen, showclient_t show) return false; } +struct tasklist_hook_data { + draw_context_t *ctx; + area_t *area; +}; + +static void +tasklist_markup_on_elem(markup_parser_data_t *p, const char *elem, + const char **names, const char **values) +{ + struct tasklist_hook_data *data = p->priv; + draw_context_t *ctx = data->ctx; + + assert (!strcmp(elem, "bg")); + for (; *names; names++, values++) { + if(!a_strcmp(*names, "color")) + { + xcolor_t bg_color; + xcolor_new(ctx->connection, ctx->phys_screen, *values, &bg_color); + draw_rectangle(ctx, *data->area, 1.0, true, bg_color); + break; + } + } +} + + static int tasklist_draw(draw_context_t *ctx, int screen, widget_node_t *w, @@ -72,11 +97,8 @@ tasklist_draw(draw_context_t *ctx, int screen, tasklist_data_t *d = w->widget->data; area_t area; char *text; - int n = 0, i = 0, box_width = 0, icon_width = 0, box_width_rest = 0, j = 0; + int n = 0, i = 0, box_width = 0, icon_width = 0, box_width_rest = 0; netwm_icon_t *icon; - markup_parser_data_t p; - const char *elements[] = { "bg", NULL }; - xcolor_t bg_color; draw_image_t *image; if(used >= ctx->width) @@ -114,6 +136,14 @@ tasklist_draw(draw_context_t *ctx, int screen, if(d->show_icons) { + static char const * const elements[] = { "bg", NULL }; + struct tasklist_hook_data data = { .ctx = ctx, .area = &area }; + markup_parser_data_t p = { + .elements = elements, + .on_element = &tasklist_markup_on_elem, + .priv = &data, + }; + /* draw a background for icons */ area.x = w->area.x + box_width * i; area.y = w->area.y; @@ -122,15 +152,8 @@ tasklist_draw(draw_context_t *ctx, int screen, /* Actually look for the proper background color, since * otherwise the background statusbar color is used instead */ - markup_parser_data_init(&p, elements, NULL, countof(elements)); - if(markup_parse(&p, text, a_strlen(text)) && p.attribute_names[0]) - for(j = 0; p.attribute_names[0][j]; j++) - if(!a_strcmp(p.attribute_names[0][j], "color")) - { - xcolor_new(ctx->connection, ctx->phys_screen, p.attribute_values[0][j], &bg_color); - draw_rectangle(ctx, area, 1.0, true, bg_color); - break; - } + markup_parser_data_init(&p); + markup_parse(&p, text, a_strlen(text)); markup_parser_data_wipe(&p); if((image = draw_image_new(c->icon_path)))