summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathan Scott <nathans@redhat.com>2021-07-12 16:51:19 +1000
committerNathan Scott <nathans@redhat.com>2021-07-14 11:58:46 +1000
commitbf853addc3e4b855cc8fb09a6558733d12548fbe (patch)
treef09dce3297b7fe05f0169d6f3ea3a5ddede4e9b7
parenta4764902829d2a25a0e27c31824274362e11adea (diff)
Ensure PCP dynamic metric configuration definition uniqueness
It can happen that pcp-htop is presented multiple definitions of the same dynamic meter, e.g. if /etc/pcp/htop/meters has a definition matching one in ~/.config/htop/meters - instead of exiting with a duplicate metric error provide more meaningful diagnostics (on close) and also just skip over such entries. System files override home directories which overrides those found below the current working directory. Also fix the derived metric error diagnostic; because this is using CRT_fatalError, which is like perror(3), we must give a meaningful prefix (like program name) at the string end.
-rw-r--r--DynamicMeter.c10
-rw-r--r--DynamicMeter.h4
-rw-r--r--Header.c4
-rw-r--r--pcp/PCPDynamicMeter.c48
4 files changed, 38 insertions, 28 deletions
diff --git a/DynamicMeter.c b/DynamicMeter.c
index 605bbc3d..0b4063f2 100644
--- a/DynamicMeter.c
+++ b/DynamicMeter.c
@@ -48,16 +48,16 @@ static void DynamicMeter_compare(ht_key_t key, void* value, void* data) {
}
}
-bool DynamicMeter_search(const ProcessList* pl, const char* name, unsigned int* key) {
+bool DynamicMeter_search(Hashtable* dynamics, const char* name, unsigned int* key) {
DynamicIterator iter = { .key = 0, .name = name, .found = false };
- if (pl->dynamicMeters)
- Hashtable_foreach(pl->dynamicMeters, DynamicMeter_compare, &iter);
+ if (dynamics)
+ Hashtable_foreach(dynamics, DynamicMeter_compare, &iter);
*key = iter.key;
return iter.found;
}
-const char* DynamicMeter_lookup(const ProcessList* pl, unsigned int key) {
- const DynamicMeter* meter = Hashtable_get(pl->dynamicMeters, key);
+const char* DynamicMeter_lookup(Hashtable* dynamics, unsigned int key) {
+ const DynamicMeter* meter = Hashtable_get(dynamics, key);
return meter ? meter->name : NULL;
}
diff --git a/DynamicMeter.h b/DynamicMeter.h
index e723503b..27364d88 100644
--- a/DynamicMeter.h
+++ b/DynamicMeter.h
@@ -16,9 +16,9 @@ typedef struct DynamicMeter_ {
Hashtable* DynamicMeters_new(void);
-const char* DynamicMeter_lookup(const ProcessList* pl, unsigned int param);
+const char* DynamicMeter_lookup(Hashtable* dynamics, unsigned int param);
-bool DynamicMeter_search(const ProcessList* pl, const char* name, unsigned int* key);
+bool DynamicMeter_search(Hashtable* dynamics, const char* name, unsigned int* key);
extern const MeterClass DynamicMeter_class;
diff --git a/Header.c b/Header.c
index 3b576a73..8903fc33 100644
--- a/Header.c
+++ b/Header.c
@@ -75,7 +75,7 @@ void Header_writeBackToSettings(const Header* this) {
const Meter* meter = (Meter*) Vector_get(vec, i);
char* name;
if (meter->param && As_Meter(meter) == &DynamicMeter_class) {
- const char* dynamic = DynamicMeter_lookup(this->pl, meter->param);
+ const char* dynamic = DynamicMeter_lookup(this->pl->dynamicMeters, meter->param);
xAsprintf(&name, "%s(%s)", As_Meter(meter)->name, dynamic);
} else if (meter->param && As_Meter(meter) == &CPUMeter_class) {
xAsprintf(&name, "%s(%u)", As_Meter(meter)->name, meter->param);
@@ -101,7 +101,7 @@ bool Header_addMeterByName(Header* this, const char* name, int column) {
if ((end = strrchr(dynamic, ')')) == NULL)
return false; // indicate htoprc parse failure
*end = '\0';
- if (!DynamicMeter_search(this->pl, dynamic, &param))
+ if (!DynamicMeter_search(this->pl->dynamicMeters, dynamic, &param))
return false; // indicates name lookup failure
}
}
diff --git a/pcp/PCPDynamicMeter.c b/pcp/PCPDynamicMeter.c
index a89c1328..ac69d5a7 100644
--- a/pcp/PCPDynamicMeter.c
+++ b/pcp/PCPDynamicMeter.c
@@ -65,8 +65,8 @@ static void PCPDynamicMeter_parseMetric(PCPDynamicMeters* meters, PCPDynamicMete
if (pmRegisterDerivedMetric(metric->name, value, &error) < 0) {
char* note;
xAsprintf(&note,
- "%s: failed to parse expression in %s at line %u\n%s\n",
- pmGetProgname(), path, line, error);
+ "%s: failed to parse expression in %s at line %u\n%s\n%s",
+ pmGetProgname(), path, line, error, pmGetProgname());
free(error);
errno = EINVAL;
CRT_fatalError(note);
@@ -106,20 +106,17 @@ static void PCPDynamicMeter_parseMetric(PCPDynamicMeters* meters, PCPDynamicMete
}
// Ensure a valid name for use in a PCP metric name and in htoprc
-static void PCPDynamicMeter_validateMeterName(char* key, const char* path, unsigned int line) {
+static bool PCPDynamicMeter_validateMeterName(char* key, const char* path, unsigned int line) {
char* p = key;
char* end = strrchr(key, ']');
if (end) {
*end = '\0';
} else {
- char* note;
- xAsprintf(&note,
- "%s: no closing brace on meter name at %s line %u\n\"%s\"",
+ fprintf(stderr,
+ "%s: no closing brace on meter name at %s line %u\n\"%s\"\n",
pmGetProgname(), path, line, key);
- errno = EINVAL;
- CRT_fatalError(note);
- free(note);
+ return false;
}
while (*p) {
@@ -133,16 +130,23 @@ static void PCPDynamicMeter_validateMeterName(char* key, const char* path, unsig
p++;
}
if (*p != '\0') { /* badness */
- char* note;
- xAsprintf(&note,
- "%s: invalid meter name at %s line %u\n\"%s\"",
+ fprintf(stderr,
+ "%s: invalid meter name at %s line %u\n\"%s\"\n",
pmGetProgname(), path, line, key);
- errno = EINVAL;
- CRT_fatalError(note);
- free(note);
- } else { /* overwrite closing brace */
- *p = '\0';
+ return false;
}
+ return true;
+}
+
+// Ensure a meter name has not been defined previously
+static bool PCPDynamicMeter_uniqueName(char* key, const char* path, unsigned int line, PCPDynamicMeters* meters) {
+ unsigned int param = 0;
+ if (DynamicMeter_search(meters->table, key, &param) == false)
+ return true;
+
+ fprintf(stderr, "%s: duplicate name at %s line %u: \"%s\", ignored\n",
+ pmGetProgname(), path, line, key);
+ return false;
}
static PCPDynamicMeter* PCPDynamicMeter_new(PCPDynamicMeters* meters, const char* name) {
@@ -159,6 +163,7 @@ static void PCPDynamicMeter_parseFile(PCPDynamicMeters* meters, const char* path
PCPDynamicMeter* meter = NULL;
unsigned int lineno = 0;
+ bool ok = true;
for (;;) {
char* line = String_readLine(file);
if (!line)
@@ -182,8 +187,13 @@ static void PCPDynamicMeter_parseFile(PCPDynamicMeters* meters, const char* path
char* key = String_trim(config[0]);
char* value = n > 1 ? String_trim(config[1]) : NULL;
if (key[0] == '[') { /* new section heading - i.e. new meter */
- PCPDynamicMeter_validateMeterName(key+1, path, lineno);
- meter = PCPDynamicMeter_new(meters, key+1);
+ ok = PCPDynamicMeter_validateMeterName(key+1, path, lineno);
+ if (ok)
+ ok = PCPDynamicMeter_uniqueName(key+1, path, lineno, meters);
+ if (ok)
+ meter = PCPDynamicMeter_new(meters, key+1);
+ } else if (!ok) {
+ ; /* skip this one, we're looking for a new header */
} else if (value && String_eq(key, "caption")) {
char* caption = String_cat(value, ": ");
free_and_xStrdup(&meter->super.caption, caption);