Skip to content

Commit

Permalink
test: add tests for set_from_string and fix it's problems
Browse files Browse the repository at this point in the history
Most problems appear when setting the same value multiple times. Fixed
the frees and made checks more thorough.
  • Loading branch information
fwsmit committed Mar 13, 2021
1 parent bbbfce0 commit b627ee0
Show file tree
Hide file tree
Showing 4 changed files with 559 additions and 41 deletions.
80 changes: 45 additions & 35 deletions src/option_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>

#include "x11/x.h"
#include "dunst.h"
Expand Down Expand Up @@ -73,6 +74,7 @@ int string_parse_mouse_action_list(char **s, void *ret_void)
if (!string_parse_enum(&mouse_action_enum_data, s[i], *ret + i)) {
LOG_W("Unknown mouse action value: '%s'", s[i]);
g_free(*ret);
*ret = NULL;
return false;
}
}
Expand Down Expand Up @@ -105,17 +107,28 @@ int string_parse_sepcolor(void *data, const char *s, void *ret)
bool is_enum = string_parse_enum(data, s, &type);
if (is_enum) {
sep_color->type = type;
g_free(sep_color->sep_color);
sep_color->sep_color = NULL;
return true;
} else {
if (STR_FULL(s)) {
sep_color->type = SEP_CUSTOM;
g_free(sep_color->sep_color);
sep_color->sep_color = g_strdup(s); // TODO check if this is a color?
return true;
} else {
if (STR_EMPTY(s)) {
LOG_W("Sep color is empty, make sure to quote the value if it's a color.");
return false;
}
if (s[0] != '#') {
LOG_W("Sep color should start with a '#'");
return false;
}
if (strlen(s) < 4) {
LOG_W("Make sure the sep color is formatted correctly");
return false;
}
// TODO add more checks for if the color is valid

sep_color->type = SEP_CUSTOM;
g_free(sep_color->sep_color);
sep_color->sep_color = g_strdup(s);
return true;
}
}

Expand Down Expand Up @@ -243,30 +256,6 @@ const char *next_section(const char *section)
return NULL;
}

int str_to_bool(const char *value){
int def = -1;
if (value) {
switch (value[0]) {
case 'y':
case 'Y':
case 't':
case 'T':
case '1':
return true;
case 'n':
case 'N':
case 'f':
case 'F':
case '0':
return false;
default:
return def;
}
} else {
return def;
}
}

int get_setting_id(const char *key, const char *section) {
int error_code = 0;
int partial_match_id = -1;
Expand Down Expand Up @@ -303,7 +292,7 @@ int get_setting_id(const char *key, const char *section) {
return -1;
}

bool set_from_string(void *target, struct setting setting, char *value) {
bool set_from_string(void *target, struct setting setting, const char *value) {
GError *error = NULL;

if (!strlen(value)) {
Expand All @@ -315,11 +304,28 @@ bool set_from_string(void *target, struct setting setting, char *value) {
// target instead
switch (setting.type) {
case TYPE_INT:
// TODO use strtol or strtoimax instead to get better
// error handling
*(int*) target = atoi(value);
return true;
case TYPE_BOOLEAN:
*(bool*) target = str_to_bool(value);
return true;
case TYPE_BOOLEAN: ;
// this is needed, since string_parse_enum assumses a
// variable of size int is passed
int temp_target = -1;
bool success1 = string_parse_enum(boolean_enum_data, value, &temp_target);

if (!success1) LOG_W("Unknown %s value: '%s'. It should be a valid boolean",
setting.name, value);

if (temp_target < 0 || temp_target > 1)
{
// should not happen if boolean_enum_data is correct
LOG_W("TYPE_BOOLEAN out of range");
return false;
}

*(bool*) target = (bool) temp_target;
return success1;
case TYPE_STRING:
g_free(*(char**) target);
*(char**) target = g_strdup(value);
Expand Down Expand Up @@ -361,7 +367,11 @@ bool set_from_string(void *target, struct setting setting, char *value) {
}
return true;
case TYPE_TIME: ;
*(gint64*) target = string_to_time(value);
gint64 temp_target2 = string_to_time(value);
if (errno != 0) {
return false;
}
*(gint64*) target = temp_target2;
return true;
case TYPE_GEOMETRY:
*(struct geometry*) target = x_parse_geometry(value);
Expand Down
22 changes: 22 additions & 0 deletions src/settings_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,28 @@ static struct string_to_enum_def verbosity_enum_data[] = {
ENUM_END,
};

static struct string_to_enum_def boolean_enum_data[] = {
{"True", true },
{"true", true },
{"On", true },
{"on", true },
{"Yes", true },
{"yes", true },
{"1", true },
{"False", false },
{"false", false },
{"Off", false },
{"off", false },
{"No", false },
{"no", false },
{"0", false },
{"n", false },
{"y", false },
{"N", false },
{"Y", true },
ENUM_END,
};

static struct string_to_enum_def horizontal_alignment_enum_data[] = {
{"left", ALIGN_LEFT },
{"center", ALIGN_CENTER },
Expand Down
25 changes: 19 additions & 6 deletions src/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,25 +171,35 @@ gint64 string_to_time(const char *string)

errno = 0;
char *endptr;
gint64 val = strtoll(string, &endptr, 10);
gint64 val = strtol(string, &endptr, 10);

if (errno != 0) {
LOG_W("Time: '%s': %s.", string, strerror(errno));
return 0;
} else if (string == endptr) {
errno = EINVAL;
LOG_W("Time: '%s': No digits found.", string);
return 0;
} else if (errno != 0 && val == 0) {
LOG_W("Time: '%s': Unknown error.", string);
return 0;
} else if (errno == 0 && !*endptr) {
} else if (val < -1) {
// most times should not be negative, but show_age_threshhold
// can be -1
LOG_W("Time: '%s': Time should be positive (-1 is allowed too sometimes)",
string);
errno = EINVAL;
}
else if (errno == 0 && !*endptr) {
return S2US(val);
}

// endptr may point to a separating space
while (isspace(*endptr))
endptr++;

if (val < 0) {
LOG_W("Time: '%s' signal value -1 should not have a suffix", string);
errno = EINVAL;
return 0;
}

if (STRN_EQ(endptr, "ms", 2))
return val * 1000;
else if (STRN_EQ(endptr, "s", 1))
Expand All @@ -201,7 +211,10 @@ gint64 string_to_time(const char *string)
else if (STRN_EQ(endptr, "d", 1))
return S2US(val) * 60 * 60 * 24;
else
{
errno = EINVAL;
return 0;
}
}

/* see utils.h */
Expand Down
Loading

0 comments on commit b627ee0

Please sign in to comment.