Skip to content

Commit

Permalink
Fix and test graph parsing (#1447)
Browse files Browse the repository at this point in the history 
* specials: comment all parser cases

* specials: scan_graph: early-return if args is null

* specials: fix logic in scan_graph

* tests: add scan_graph parsing test

* specials: separate command parsing from graph parsing

this simplifies the implementation of graph parsing and makes it less ambiguous to parse graphs in contexts where commands are not expected

* specials: tweak comments in scan_graph

* tests: update for scan_command separation

---------

Co-authored-by: bi4k8 <bi4k8@github>
  • Loading branch information
@bi4k8
bi4k8 and bi4k8 authored Mar 6, 2023
1 parent 7b64e0a commit 566fbe7
Show file tree
Hide file tree
Showing 10 changed files with 354 additions and 154 deletions.
5 changes: 1 addition & 4 deletions src/common.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,7 @@ void print_no_update(struct text_object *obj, char *p,

#ifdef BUILD_GUI
void scan_loadgraph_arg(struct text_object *obj, const char *arg) {
char *buf = nullptr;

buf = scan_graph(obj, arg, 0);
free_and_zero(buf);
scan_graph(obj, arg, 0);
}

double loadgraphval(struct text_object *obj) {
Expand Down
21 changes: 7 additions & 14 deletions src/core.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,11 +725,9 @@ struct text_object *construct_text_object(char *s, const char *arg, long line,
DBGP2("Adding $cpubar for CPU %d", obj->data.i);
#ifdef BUILD_GUI
END OBJ(cpugraph, &update_cpu_usage) get_cpu_count();
char *buf = nullptr;
SCAN_CPU(arg, obj->data.i);
buf = scan_graph(obj, arg, 1);
scan_graph(obj, arg, 1);
DBGP2("Adding $cpugraph for CPU %d", obj->data.i);
free_and_zero(buf);
obj->callbacks.graphval = &cpu_barval;
obj->callbacks.free = &free_cpu;
END OBJ(loadgraph, &update_load_average) scan_loadgraph_arg(obj, arg);
Expand Down Expand Up @@ -1244,13 +1242,9 @@ struct text_object *construct_text_object(char *s, const char *arg, long line,
END OBJ(memwithbuffersbar, &update_meminfo) scan_bar(obj, arg, 1);
obj->callbacks.barval = &mem_with_buffers_barval;
#ifdef BUILD_GUI
END OBJ(memgraph, &update_meminfo) char *buf = nullptr;
buf = scan_graph(obj, arg, 1);
free_and_zero(buf);
END OBJ(memgraph, &update_meminfo) scan_graph(obj, arg, 1);
obj->callbacks.graphval = &mem_barval;
END OBJ(memwithbuffersgraph, &update_meminfo) char *buf = nullptr;
buf = scan_graph(obj, arg, 1);
free_and_zero(buf);
END OBJ(memwithbuffersgraph, &update_meminfo) scan_graph(obj, arg, 1);
obj->callbacks.graphval = &mem_with_buffers_barval;
#endif /* BUILD_GUI*/
#ifdef HAVE_SOME_SOUNDCARD_H
Expand Down Expand Up @@ -1829,8 +1823,9 @@ struct text_object *construct_text_object(char *s, const char *arg, long line,
END OBJ_ARG(
lua_graph, nullptr,
"lua_graph needs arguments: <function name> [height],[width] [gradient "
"colour 1] [gradient colour 2] [scale] [-t] [-l]") char *buf = nullptr;
buf = scan_graph(obj, arg, 100);
"colour 1] [gradient colour 2] [scale] [-t] [-l]") auto [buf, skip] =
scan_command(arg);
scan_graph(obj, arg + skip, 100);
if (buf != nullptr) {
obj->data.s = buf;
} else {
Expand Down Expand Up @@ -1967,9 +1962,7 @@ struct text_object *construct_text_object(char *s, const char *arg, long line,
END OBJ(apcupsd_loadbar, &update_apcupsd) scan_bar(obj, arg, 100);
obj->callbacks.barval = &apcupsd_loadbarval;
#ifdef BUILD_GUI
END OBJ(apcupsd_loadgraph, &update_apcupsd) char *buf = nullptr;
buf = scan_graph(obj, arg, 100);
free_and_zero(buf);
END OBJ(apcupsd_loadgraph, &update_apcupsd) scan_graph(obj, arg, 100);
obj->callbacks.graphval = &apcupsd_loadbarval;
END OBJ(apcupsd_loadgauge, &update_apcupsd) scan_gauge(obj, arg, 100);
obj->callbacks.gaugeval = &apcupsd_loadbarval;
Expand Down
4 changes: 2 additions & 2 deletions src/diskio.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ void print_diskio_write(struct text_object *obj, char *p,

#ifdef BUILD_GUI
void parse_diskiograph_arg(struct text_object *obj, const char *arg) {
char *buf = nullptr;
buf = scan_graph(obj, arg, 0);
auto [buf, skip] = scan_command(arg);
scan_graph(obj, arg + skip, 0);

obj->data.opaque = prepare_diskio_stat(dev_name(buf));
free_and_zero(buf);
Expand Down
4 changes: 3 additions & 1 deletion src/exec.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,9 @@ void scan_exec_arg(struct text_object *obj, const char *arg,
} else if ((execflag & EF_GAUGE) != 0u) {
cmd = scan_gauge(obj, cmd, 100);
} else if ((execflag & EF_GRAPH) != 0u) {
cmd = scan_graph(obj, cmd, 100);
auto [buf, skip] = scan_command(cmd);
scan_graph(obj, cmd + skip, 100);
cmd = buf;
if (cmd == nullptr) {
NORM_ERR("error parsing arguments to execgraph object");
}
Expand Down
4 changes: 2 additions & 2 deletions src/net_stat.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ void print_v6addrs(struct text_object *obj, char *p, unsigned int p_max_size) {
void parse_net_stat_graph_arg(struct text_object *obj, const char *arg,
void *free_at_crash) {
/* scan arguments and get interface name back */
char *buf = nullptr;
buf = scan_graph(obj, arg, 0);
auto [buf, skip] = scan_command(arg);
scan_graph(obj, arg + skip, 0);

// default to DEFAULTNETDEV
if (buf != nullptr) {
Expand Down
8 changes: 5 additions & 3 deletions src/nvidia.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,11 @@ int set_nvidia_query(struct text_object *obj, const char *arg,
case BAR:
arg = scan_bar(obj, arg, 100);
break;
case GRAPH:
arg = scan_graph(obj, arg, 100);
break;
case GRAPH: {
auto [buf, skip] = scan_command(arg);
scan_graph(obj, arg + skip, 100);
arg = buf;
} break;
case GAUGE:
arg = scan_gauge(obj, arg, 100);
break;
Expand Down
241 changes: 114 additions & 127 deletions src/specials.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,45 @@ void apply_graph_colours(struct graph *g, const char *first_colour_name,
g->colours_set = true;
}

/**
* parses a possibly-quoted command from the prefix of a string.
* @param[in] s argument string to parse
* @return pair of the command and the number of bytes parsed (counting quotes).
* The command will be nullptr if the string to parse started with a
* digit or an unpaired double-quote.
**/
std::pair<char *, size_t> scan_command(const char *s) {
if (s == nullptr) return {nullptr, 0};

/* unquoted commands are not permitted to begin with digits. This allows
distinguishing a commands from a position in execgraph objects */
if (isdigit(*s)) { return {nullptr, 0}; }

/* extract double-quoted command in case of execgraph */
if (*s == '"') {
size_t _size;
char *_ptr;
if (((_ptr = const_cast<char *>(strrchr(s, '"'))) != nullptr) &&
_ptr != s) {
_size = _ptr - s - 1;
} else {
NORM_ERR("mismatched double-quote in execgraph object");
return {nullptr, 0};
}

char *quoted_cmd = static_cast<char *>(malloc(_size + 1));
quoted_cmd[0] = '\0';
strncpy(quoted_cmd, s + 1, _size);
quoted_cmd[_size] = '\0';
return {quoted_cmd, _size + 2};
} else {
size_t len;
for (len = 0; s[len] != '\0' && !isspace(s[len]); len++)
;
return {strndup(s, len), len};
}
}

/**
* parses for [height,width] [color1 color2] [scale] [-t] [-l]
*
Expand All @@ -208,158 +247,106 @@ void apply_graph_colours(struct graph *g, const char *first_colour_name,
* @param[out] obj struct in which to save width, height and other options
* @param[in] args argument string to parse
* @param[in] defscale default scale if no scale argument given
* @return string to the command argument, nullptr if argument didn't start with
* a string, but a number or if invalid argument string
* @return whether parsing was successful
**/
char *scan_graph(struct text_object *obj, const char *args, double defscale) {
char quoted_cmd[1024] = {'\0'}; /* double-quoted execgraph command */
char argstr[1024] = {'\0'}; /* args minus quoted_cmd */
char buf[1024] = {'\0'}; /* first unquoted string argument in argstr */
bool scan_graph(struct text_object *obj, const char *argstr, double defscale) {
char first_colour_name[1024] = {'\0'};
char last_colour_name[1024] = {'\0'};

auto *g = static_cast<struct graph *>(malloc(sizeof(struct graph)));
memset(g, 0, sizeof(struct graph));
obj->special_data = g;

/* zero width means all space that is available */
g->id = ++graph_count;
/* zero width means all space that is available */
g->width = default_graph_width.get(*state);
g->height = default_graph_height.get(*state);
g->colours_set = false;
g->first_colour = Colour();
g->last_colour = Colour();
g->scale = defscale;
g->tempgrad = FALSE;
if (args != nullptr) {
/* extract double-quoted command in case of execgraph */
if (*args == '"') {
char *_ptr;
size_t _size;
if (((_ptr = const_cast<char *>(strrchr(args, '"'))) != nullptr) &&
_ptr != args) {
_size = _ptr - args - 1;
} else {
NORM_ERR("mismatched double-quote in execgraph object");
return nullptr;
}

_size = _size < 1024 ? _size : 1023;
strncpy(quoted_cmd, args + 1, _size);
quoted_cmd[_size] = '\0';
if (argstr == nullptr) return false;

/* copy everything after the last quote into argstr */
if (_size + 2 < strlen(args)) { strncpy(argstr, args + _size + 2, 1023); }
} else {
/* redundant, but simplifies the code below */
strncpy(argstr, args, 1023);
}
/* set tempgrad to true if '-t' specified.
* It doesn't matter where the argument is exactly. */
if ((strstr(argstr, " " TEMPGRAD) != nullptr) ||
strncmp(argstr, TEMPGRAD, strlen(TEMPGRAD)) == 0) {
g->tempgrad = TRUE;
}
/* set showlog flag if '-l' specified.
* It doesn't matter where the argument is exactly. */
if ((strstr(argstr, " " LOGGRAPH) != nullptr) ||
strncmp(argstr, LOGGRAPH, strlen(LOGGRAPH)) == 0) {
g->flags |= SF_SHOWLOG;
}

/* set tempgrad to true, if '-t' specified.
* It doesn#t matter where the argument is exactly. */
if ((strstr(argstr, " " TEMPGRAD) != nullptr) ||
strncmp(argstr, TEMPGRAD, strlen(TEMPGRAD)) == 0) {
g->tempgrad = TRUE;
}
/* set showlog-flag, if '-l' specified
* It doesn#t matter where the argument is exactly. */
if ((strstr(argstr, " " LOGGRAPH) != nullptr) ||
strncmp(argstr, LOGGRAPH, strlen(LOGGRAPH)) == 0) {
g->flags |= SF_SHOWLOG;
}
/* all the following functions try to interpret the beginning of a
* a string with different format strings. If successful, they return from
* the function */

/* interpret the beginning(!) of the argument string as:
* '[height],[width] [color1] [color2] [scale]'
* This means parameters like -t and -l may not be in the beginning */
if (sscanf(argstr, "%d,%d %s %s %lf", &g->height, &g->width,
first_colour_name, last_colour_name, &g->scale) == 5) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return true;
}
g->height = default_graph_height.get(*state);
g->width = default_graph_width.get(*state);
first_colour_name[0] = '\0';
last_colour_name[0] = '\0';
g->scale = defscale;

/* all the following functions try to interpret the beginning of a
* a string with different formaters. If successfully the return from
* this whole function */

/* interpret the beginning(!) of the argument string as:
* '[height],[width] [color1] [color2] [scale]'
* This means parameters like -t and -l may not be in the beginning */
if (sscanf(argstr, "%d,%d %s %s %lf", &g->height, &g->width,
first_colour_name, last_colour_name, &g->scale) == 5) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
/* [height],[width] [color1] [color2] */
g->scale = defscale;
if (sscanf(argstr, "%d,%d %s %s", &g->height, &g->width, first_colour_name,
last_colour_name) == 4) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
/* [command] [height],[width] [color1] [color2] [scale] */
if (sscanf(argstr, "%1023s %d,%d %s %s %lf", buf, &g->height, &g->width,
first_colour_name, last_colour_name, &g->scale) == 6) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return strndup(buf, text_buffer_size.get(*state));
}
g->scale = defscale;
if (sscanf(argstr, "%1023s %d,%d %s %s", buf, &g->height, &g->width,
first_colour_name, last_colour_name) == 5) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return strndup(buf, text_buffer_size.get(*state));
}
/* [height],[width] [color1] [color2] */
if (sscanf(argstr, "%d,%d %s %s", &g->height, &g->width, first_colour_name,
last_colour_name) == 4) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return true;
}
g->height = default_graph_height.get(*state);
g->width = default_graph_width.get(*state);
first_colour_name[0] = '\0';

buf[0] = '\0';
g->height = default_graph_height.get(*state);
g->width = default_graph_width.get(*state);
if (sscanf(argstr, "%s %s %lf", first_colour_name, last_colour_name,
&g->scale) == 3) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
g->scale = defscale;
if (sscanf(argstr, "%s %s", first_colour_name, last_colour_name) == 2) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
if (sscanf(argstr, "%1023s %s %s %lf", buf, first_colour_name,
last_colour_name, &g->scale) == 4) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return strndup(buf, text_buffer_size.get(*state));
}
g->scale = defscale;
if (sscanf(argstr, "%1023s %s %s", buf, first_colour_name,
last_colour_name) == 3) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return strndup(buf, text_buffer_size.get(*state));
}
/* [height],[width] [scale] */
if (sscanf(argstr, "%d,%d %lf", &g->height, &g->width, &g->scale) == 3) {
return true;
}
g->height = default_graph_height.get(*state);
g->width = default_graph_width.get(*state);

buf[0] = '\0';
first_colour_name[0] = '\0';
last_colour_name[0] = '\0';
if (sscanf(argstr, "%d,%d %lf", &g->height, &g->width, &g->scale) == 3) {
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
g->scale = defscale;
if (sscanf(argstr, "%d,%d", &g->height, &g->width) == 2) {
return *quoted_cmd != 0
? strndup(quoted_cmd, text_buffer_size.get(*state))
: nullptr;
}
if (sscanf(argstr, "%1023s %d,%d %lf", buf, &g->height, &g->width,
&g->scale) < 4) {
g->scale = defscale;
// TODO(brenden): check the return value and throw an error?
sscanf(argstr, "%1023s %d,%d", buf, &g->height, &g->width);
}
/* [height],[width] */
if (sscanf(argstr, "%d,%d", &g->height, &g->width) == 2) { return true; }
g->height = default_graph_height.get(*state);
g->width = default_graph_width.get(*state);

if ((*quoted_cmd == 0) && (*buf == 0)) { return nullptr; }
return strndup(*quoted_cmd != 0 ? quoted_cmd : buf,
text_buffer_size.get(*state));
/* [height], */
char comma;
if (sscanf(argstr, "%d%[,]", &g->height, &comma) == 2) { return true; }
g->height = default_graph_height.get(*state);

/* [color1] [color2] [scale] */
if (sscanf(argstr, "%s %s %lf", first_colour_name, last_colour_name,
&g->scale) == 3) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return true;
}
first_colour_name[0] = '\0';
last_colour_name[0] = '\0';
g->scale = defscale;

/* [color1] [color2] */
if (sscanf(argstr, "%s %s", first_colour_name, last_colour_name) == 2) {
apply_graph_colours(g, first_colour_name, last_colour_name);
return true;
}

return nullptr;
/* [scale] */
if (sscanf(argstr, "%lf", &g->scale) == 1) { return true; }

return true;
}
#endif /* BUILD_GUI */

Expand Down
Loading

0 comments on commit 566fbe7

Please sign in to comment.