Fix and test graph parsing (#1447)

* 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>
This commit is contained in:
bi4k8 2023-03-06 00:25:56 +00:00 committed by GitHub
parent 7b64e0a349
commit 566fbe7187
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 359 additions and 159 deletions

View File

@ -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) {

View File

@ -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);
@ -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
@ -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 {
@ -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;

View File

@ -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);

View File

@ -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");
}

View File

@ -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) {

View File

@ -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;

View File

@ -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]
*
@ -208,13 +247,9 @@ 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'};
@ -222,8 +257,8 @@ char *scan_graph(struct text_object *obj, const char *args, double defscale) {
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;
@ -231,135 +266,87 @@ char *scan_graph(struct text_object *obj, const char *args, double defscale) {
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;
}
/* 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));
}
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));
}
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);
}
if ((*quoted_cmd == 0) && (*buf == 0)) { return nullptr; }
return strndup(*quoted_cmd != 0 ? quoted_cmd : buf,
text_buffer_size.get(*state));
/* 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;
}
return nullptr;
/* 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;
/* [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';
/* [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);
/* [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);
/* [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;
}
/* [scale] */
if (sscanf(argstr, "%lf", &g->scale) == 1) { return true; }
return true;
}
#endif /* BUILD_GUI */

View File

@ -29,6 +29,7 @@
#ifndef _SPECIALS_H
#define _SPECIALS_H
#include <tuple>
#include "colours.h"
/* special stuff in text_buffer */
@ -92,7 +93,8 @@ const char *scan_bar(struct text_object *, const char *, double);
const char *scan_gauge(struct text_object *, const char *, double);
#ifdef BUILD_GUI
void scan_font(struct text_object *, const char *);
char *scan_graph(struct text_object *, const char *, double);
std::pair<char *, size_t> scan_command(const char *);
bool scan_graph(struct text_object *, const char *, double);
void scan_tab(struct text_object *, const char *);
void scan_stippled_hr(struct text_object *, const char *);

View File

@ -18,6 +18,7 @@ set(test_srcs ${test_srcs} test-core.cc)
set(test_srcs ${test_srcs} test-diskio.cc)
set(test_srcs ${test_srcs} test-fs.cc)
set(test_srcs ${test_srcs} test-gradient.cc)
set(test_srcs ${test_srcs} test-graph.cc)
set(test_srcs ${test_srcs} test-colours.cc)
add_executable(test-conky test-common.cc ${test_srcs})

216
tests/test-graph.cc Normal file
View File

@ -0,0 +1,216 @@
/*
*
* Conky, a system monitor, based on torsmo
*
* Any original torsmo code is licensed under the BSD license
*
* All code written since the fork of torsmo is licensed under the GPL
*
* Please see COPYING for details
*
* Copyright (c) 2005-2021 Brenden Matthews, Philip Kovacs, et. al.
* (see AUTHORS)
* All rights reserved.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
#include <tuple>
#include "catch2/catch.hpp"
#include "conky.h"
#include "lua-config.hh"
#include "specials.h"
#ifdef BUILD_GUI
#define SF_SHOWLOG (1 << 1)
// Specific value doesn't matter, but we check if the same one comes back
constexpr double default_scale = M_PI;
constexpr double default_width = 0;
constexpr double default_height = 25;
struct graph {
int id;
char flags;
int width, height;
bool colours_set;
Colour first_colour, last_colour;
double scale;
char tempgrad;
};
static std::pair<struct graph, bool> test_parse(const char *s) {
struct text_object obj;
bool result = scan_graph(&obj, s, default_scale);
auto g = static_cast<struct graph *>(obj.special_data);
struct graph graph = *g;
free(g);
return {graph, result};
}
std::string unquote(const std::string &s) {
auto out = s;
out.erase(remove(out.begin(), out.end(), '\"'), out.end());
return out;
}
TEST_CASE("scan_graph correctly parses input strings") {
state = std::make_unique<lua::state>();
conky::export_symbols(*state);
SECTION("Trivial parse") {
auto [g, success] = test_parse("");
REQUIRE(g.width == default_width);
REQUIRE(g.height == default_height);
}
/* test parsing combinations of options */
const char *size_options[][2] = {{"30", "500"}, {"80", ""}, {"", ""}};
const char *color_options[][2] = {{"orange", "blue"},
{"#deadff", "#392014"},
{"000000", "000000"},
{"", ""}};
const char *scale_options[] = {"0.5", ""};
SECTION("subset of [height,width] [color1 color2] [scale] [-t] [-l]") {
for (auto size : size_options) {
for (auto colors : color_options) {
for (auto scale : scale_options) {
bool ends_at_first_size = false;
/* build an argument string by combining the selected options */
std::string s;
if (*size[0] != '\0') {
s += size[0];
s += ",";
if (*size[1] == '\0') {
/* if the size is just a height, it has to be the end of the
* argument string */
ends_at_first_size = true;
} else {
s += size[1];
}
s += " ";
}
if (!ends_at_first_size) {
s += colors[0];
s += " ";
s += colors[1];
s += " ";
s += scale;
}
/* parse the argument string */
auto [g, success] = test_parse(s.c_str());
printf("command: %s\n", s.c_str());
/* validate parsing of each component */
if (*size[0] == '\0') {
REQUIRE(g.width == default_width);
REQUIRE(g.height == default_height);
} else {
REQUIRE(g.height == atoi(size[0]));
REQUIRE(g.width == atoi(size[1]));
}
/* if second half of size is empty, no subsequent values should be
* set
*/
if (ends_at_first_size) {
REQUIRE(g.colours_set == false);
continue;
}
if (*colors[0] == '\0') {
REQUIRE(g.colours_set == false);
} else {
REQUIRE(g.colours_set == true);
REQUIRE(g.first_colour == parse_color(colors[0]));
REQUIRE(g.last_colour == parse_color(colors[1]));
}
if (*scale == '\0') {
REQUIRE(g.scale == default_scale);
} else {
REQUIRE(g.scale == 0.5);
}
REQUIRE(g.flags == 0);
REQUIRE(g.tempgrad == 0);
}
}
}
}
SECTION("[height,width] [color1 color2] [scale] [-t] [-l]") {
auto [g, success] = test_parse("21,340 orange blue 0.5 -t -l");
REQUIRE(success);
REQUIRE(g.width == 340);
REQUIRE(g.height == 21);
REQUIRE(g.colours_set == true);
REQUIRE(g.scale == 0.5);
REQUIRE(g.flags == SF_SHOWLOG);
REQUIRE(g.tempgrad == true);
}
SECTION("-t location") {
{
auto [g, success] = test_parse("21,340 red blue 0.5");
REQUIRE(g.tempgrad == false);
}
{
auto [g, success] = test_parse("21,340 red blue 0.5");
REQUIRE(g.tempgrad == false);
}
{
auto [g, success] = test_parse("-t 21,340 red blue 0.5");
REQUIRE(g.tempgrad == true);
}
{
auto [g, success] = test_parse("21,340 -t red blue 0.5");
REQUIRE(g.tempgrad == true);
}
}
}
TEST_CASE("scan_command correctly parses input strings") {
SECTION("parse commands") {
const char *command_options[][2] = {{"\"foo bar\"", "foo bar"},
{"\"foo bar\"\tbaz", "foo bar"},
{"\"foo bar\"\nbaz", "foo bar"},
{"\"foo bar\" baz", "foo bar"},
{"one two", "one"},
{"\"ls -t\"", "ls -t"},
{"\"ls -t\" 4309", "ls -t"},
{"foo-test", "foo-test"},
{"foo-test a b c", "foo-test"},
{"", ""}};
for (auto [command, expected_parsed] : command_options) {
auto [parsed, len] = scan_command(command);
REQUIRE(std::string(parsed) == expected_parsed);
if (command[0] == '"') {
REQUIRE(len == strlen(expected_parsed) + 2);
} else {
REQUIRE(len == strlen(expected_parsed));
}
}
}
}
#endif /* BUILD_GUI */