[PATCH phosphor-event v6 2/2] Fixed Multiple associations occasionally geting truncated
OpenBMC Patches
openbmc-patches at stwcx.xyz
Thu Mar 17 16:10:26 AEDT 2016
From: Chris Austen <austenc at us.ibm.com>
After additional testing it was found that the association list gets truncated
leaving you with only one item. Upon code review it was realized that the
use of strtok manipulated a cached string and tokenized the string with NULLs.
Changed to use a temp buffer for strtok to manipulate and switched to
strtok_r since it is a best practice to ensure things are thread safe.
---
event_messaged.C | 6 ++++-
event_messaged_sdbus.c | 64 +++++++++++++++++++++++++++++++++-----------------
event_messaged_sdbus.h | 2 +-
3 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/event_messaged.C b/event_messaged.C
index 9addd3f..f2a308a 100644
--- a/event_messaged.C
+++ b/event_messaged.C
@@ -36,9 +36,13 @@ int message_delete_log(event_manager *em, uint16_t logid)
int load_existing_events(event_manager *em)
{
uint16_t id;
+ event_record_t *rec;
while ( (id = em->next_log()) != 0) {
- send_log_to_dbus(em, id);
+
+ em->open(id, &rec);
+ send_log_to_dbus(em, id, rec->association);
+ em->close(rec);
}
return 0;
diff --git a/event_messaged_sdbus.c b/event_messaged_sdbus.c
index 907ac53..af09c36 100644
--- a/event_messaged_sdbus.c
+++ b/event_messaged_sdbus.c
@@ -99,28 +99,40 @@ static int prop_message_assoc(sd_bus *bus,
messageEntry_t *m = (messageEntry_t*) userdata;
event_record_t *rec;
char *p;
- char *token;
+ char *token, *saveptr;
rec = message_record_open(m->em, m->logid);
if (!rec) {
- fprintf(stderr,"Warning missing evnet log for %lx\n", m->logid);
+ fprintf(stderr,"Warning missing event log for %lx\n", m->logid);
sd_bus_error_set(error,
SD_BUS_ERROR_FILE_NOT_FOUND,
"Could not find log file");
return -1;
}
- p = rec->association;
+ /* strtok manipulates a string. It turns out that message_record_open */
+ /* implements a caching mechcanism which means the oiginal string is */
+ /* To avoid that, I will make a copy and mess with that */
+ p = strdup(rec->association);
+
+ if (!p) {
+ /* no association string == no associations */
+ sd_bus_error_set(error,
+ SD_BUS_ERROR_NO_MEMORY,
+ "Not enough memory for association");
+ return -1;
+ }
token = strtok(p, " ");
+
if (token) {
+
r = sd_bus_message_open_container(reply, 'a', "(sss)");
if (r < 0) {
fprintf(stderr,"Error opening container %s to reply %s\n", token, strerror(-r));
}
while(token) {
-
r = sd_bus_message_append(reply, "(sss)", "fru", "event", token);
if (r < 0) {
fprintf(stderr,"Error adding properties for %s to reply %s\n", token, strerror(-r));
@@ -132,6 +144,8 @@ static int prop_message_assoc(sd_bus *bus,
r = sd_bus_message_close_container(reply);
}
+ free(p);
+
return r;
}
@@ -148,12 +162,12 @@ static int prop_message(sd_bus *bus,
messageEntry_t *m = (messageEntry_t*) userdata;
char *p;
struct tm *tm_info;
- char buffer[32];
+ char buffer[36];
event_record_t *rec;
rec = message_record_open(m->em, m->logid);
if (!rec) {
- fprintf(stderr,"Warning missing evnet log for %lx\n", m->logid);
+ fprintf(stderr,"Warning missing event log for %lx\n", m->logid);
sd_bus_error_set(error,
SD_BUS_ERROR_FILE_NOT_FOUND,
"Could not find log file");
@@ -254,7 +268,7 @@ static int method_accept_host_message(sd_bus_message *m,
logid = message_create_new_log_event(em, &rec);
if (logid)
- r = send_log_to_dbus(em, logid);
+ r = send_log_to_dbus(em, logid, rec.association);
return sd_bus_reply_method_return(m, "q", logid);
}
@@ -282,7 +296,9 @@ static int method_accept_test_message(sd_bus_message *m,
syslog(LOG_NOTICE, "%s %s (%s)", rec.severity, rec.message, rec.association);
logid = message_create_new_log_event(em, &rec);
- send_log_to_dbus(em, logid);
+
+ if (logid)
+ send_log_to_dbus(em, logid, rec.association);
return sd_bus_reply_method_return(m, "q", logid);
}
@@ -385,14 +401,16 @@ static int remove_log_from_dbus(messageEntry_t *p)
}
sd_bus_slot_unref(p->messageslot);
sd_bus_slot_unref(p->deleteslot);
- sd_bus_slot_unref(p->associationslot);
+
+ if (p->associationslot)
+ sd_bus_slot_unref(p->associationslot);
message_entry_close(p);
return 0;
}
-int send_log_to_dbus(event_manager *em, const uint16_t logid)
+int send_log_to_dbus(event_manager *em, const uint16_t logid, const char *association)
{
char loglocation[64];
int r;
@@ -428,19 +446,21 @@ int send_log_to_dbus(event_manager *em, const uint16_t logid)
message_entry_close(m);
return 0;
}
-
- r = sd_bus_add_object_vtable(bus,
- &m->associationslot,
- loglocation,
- "org.openbmc.Associations",
- recordlog_association_vtable,
- m);
- if (r < 0) {
- fprintf(stderr, "Failed to add association object for: %s %s\n",
- loglocation, strerror(-r));
- message_entry_close(m);
- return 0;
+ m->associationslot = NULL;
+ if (strlen(association) > 0) {
+ r = sd_bus_add_object_vtable(bus,
+ &m->associationslot,
+ loglocation,
+ "org.openbmc.Associations",
+ recordlog_association_vtable,
+ m);
+ if (r < 0) {
+ fprintf(stderr, "Failed to add association object for: %s %s\n",
+ loglocation, strerror(-r));
+ message_entry_close(m);
+ return 0;
+ }
}
r = sd_bus_emit_object_added(bus, loglocation);
diff --git a/event_messaged_sdbus.h b/event_messaged_sdbus.h
index 4565af8..b3cec2c 100644
--- a/event_messaged_sdbus.h
+++ b/event_messaged_sdbus.h
@@ -5,7 +5,7 @@ extern "C" {
#endif
int start_event_monitor(void);
int build_bus(event_manager *em);
- int send_log_to_dbus(event_manager *em, const uint16_t logid);
+ int send_log_to_dbus(event_manager *em, const uint16_t logid, const char* association);
void cleanup_event_monitor(void);
#ifdef __cplusplus
}
--
2.7.1
More information about the openbmc
mailing list