[PATCH] MatahariAgent: fix memory leaks, removed unused object
by russell@russellbryant.net
1) Resolve some memory leaks in places where strdup() was being
used but was not cleaned up. These strings were converted to
std::string.
2) Remove an object from MatahariAgent::init() that was unused.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/mh_agent.cpp | 83 ++++++++++++++++++++++++++++++++------------------
1 files changed, 53 insertions(+), 30 deletions(-)
diff --git a/src/lib/mh_agent.cpp b/src/lib/mh_agent.cpp
index 3f72d0f..e103dc1 100644
--- a/src/lib/mh_agent.cpp
+++ b/src/lib/mh_agent.cpp
@@ -140,13 +140,11 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
#endif
bool gssapi = false;
- char *servername = strdup(MATAHARI_BROKER);
- char *username = NULL;
- char *password = NULL;
- char *service = NULL;
- int serverport = MATAHARI_PORT;
-
- qpid::management::ConnectionSettings settings;
+ string servername;
+ string username;
+ string password;
+ string service;
+ int serverport = MATAHARI_PORT;
/* Set up basic logging */
mh_log_init(proc_name, LOG_INFO, FALSE);
@@ -168,7 +166,13 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
HKEY_LOCAL_MACHINE,
L"SYSTEM\\CurrentControlSet\\services\\Matahari",
L"Broker",
- &servername);
+ &value);
+
+ if(value) {
+ servername = value;
+ free(value);
+ value = NULL;
+ }
RegistryRead (
HKEY_LOCAL_MACHINE,
@@ -186,19 +190,38 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
HKEY_LOCAL_MACHINE,
L"SYSTEM\\CurrentControlSet\\services\\Matahari",
L"Service",
- &service);
+ &value);
+
+ if(value) {
+ service = value;
+ free(value);
+ value = NULL;
+ }
RegistryRead (
HKEY_LOCAL_MACHINE,
L"SYSTEM\\CurrentControlSet\\services\\Matahari",
L"User",
- &username);
+ &value);
+
+ if(value) {
+ username = value;
+ free(value);
+ value = NULL;
+ }
+
RegistryRead (
HKEY_LOCAL_MACHINE,
L"SYSTEM\\CurrentControlSet\\services\\Matahari",
L"Password",
- &password);
-
+ &value);
+
+ if(value) {
+ password = value;
+ free(value);
+ value = NULL;
+ }
+
#else
// Get args
@@ -218,7 +241,7 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
break;
case 's':
if (optarg) {
- service = strdup(optarg);
+ service = optarg;
} else {
print_usage(proc_name);
exit(1);
@@ -226,7 +249,7 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
break;
case 'u':
if (optarg) {
- username = strdup(optarg);
+ username = optarg;
} else {
print_usage(proc_name);
exit(1);
@@ -234,7 +257,7 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
break;
case 'P':
if (optarg) {
- password = strdup(optarg);
+ password = optarg;
} else {
print_usage(proc_name);
exit(1);
@@ -253,7 +276,7 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
break;
case 'b':
if (optarg) {
- servername = strdup(optarg);
+ servername = optarg;
} else {
print_usage(proc_name);
exit(1);
@@ -275,29 +298,27 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
}
#endif
+ if (servername.length() == 0) {
+ servername = MATAHARI_BROKER;
+ }
+
/* Re-initialize logging now that we've completed option processing */
mh_log_init(proc_name, mh_log_level, mh_log_level > LOG_INFO);
// Set up the cleanup handler for sigint
signal(SIGINT, shutdown);
- // Connect to the broker
- settings.host = servername;
- settings.port = serverport;
-
- mh_info("Connecting to Qpid broker at %s on port %d", servername, serverport);
-
// Create a v2 API options map.
qpid::types::Variant::Map options;
options["reconnect"] = bool(true);
- if (username) {
- options["username"] = username;
+ if (!username.empty()) {
+ options["username"] = username.c_str();
}
- if (password) {
- options["password"] = password;
+ if (!password.empty()) {
+ options["password"] = password.c_str();
}
- if (service) {
- options["sasl-service"] = service;
+ if (!service.empty()) {
+ options["sasl-service"] = service.c_str();
}
if (gssapi) {
options["sasl-mechanism"] = "GSSAPI";
@@ -306,6 +327,8 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
std::stringstream url;
url << servername << ":" << serverport ;
+ mh_info("Connecting to Qpid broker at %s", url.str().c_str());
+
_amqp_connection = qpid::messaging::Connection(url.str(), options);
_amqp_connection.open();
@@ -318,8 +341,8 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
/* Do any setup required by our agent */
if(this->setup(_agent_session) < 0) {
- mh_err("Failed to set up broker connection to %s on %d for %s\n",
- servername, serverport, proc_name);
+ mh_err("Failed to set up broker connection to %s for %s\n",
+ url.str().c_str(), proc_name);
return -1;
}
--
1.7.4
13 years
[PATCH] Misc cleanups in src/lib/network.c.
by russell@russellbryant.net
1) Try to make the formatting more consistent. I went with what seemed
most common in the file (4 space indent, etc.). It would be good to
decide on a style guide, though, if that has not already been done.
Also trim trailing whitespace in passing.
2) Add checking for failures of memory allocations and handle them
gracefully.
3) Use calloc() instead of malloc() + memset().
4) Use asprintf() instead of a malloc() + snprintf().
5) Use a stack buffer instead of a mallocd buffer that is local to
services_action_cancel().
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/services.c | 198 +++++++++++++++++++++++++++++++++++-----------------
1 files changed, 134 insertions(+), 64 deletions(-)
diff --git a/src/lib/services.c b/src/lib/services.c
index 43e1fcc..7cc380f 100644
--- a/src/lib/services.c
+++ b/src/lib/services.c
@@ -37,68 +37,140 @@ GHashTable *recurring_actions = NULL;
svc_action_t *services_action_create(
const char *name, const char *action, int interval, int timeout)
{
- svc_action_t *op = malloc(sizeof(svc_action_t));
- memset(op, 0, sizeof(svc_action_t));
-
- op->opaque = malloc(sizeof(svc_action_private_t));
- memset(op->opaque, 0, sizeof(svc_action_private_t));
-
- op->rsc = strdup(name);
- op->action = strdup(action);
+ svc_action_t *op;
+
+ if(!(op = calloc(1, sizeof(svc_action_t)))) {
+ return NULL;
+ }
+
+ if(!(op->opaque = calloc(1, sizeof(svc_action_private_t)))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->rsc = strdup(name))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->action = strdup(action))) {
+ goto return_cleanup;
+ }
+
op->interval = interval;
op->timeout = timeout;
- op->rclass = strdup("lsb");
- op->agent = strdup(name);
+ if(!(op->rclass = strdup("lsb"))) {
+ goto return_cleanup;
+ }
- op->id = malloc(500);
- snprintf(op->id, 500, "%s_%s_%d", name, action, interval);
+ if(!(op->agent = strdup(name))) {
+ goto return_cleanup;
+ }
+
+ if(asprintf(&op->id, "%s_%s_%d", name, action, interval) == -1) {
+ op->id = NULL;
+ goto return_cleanup;
+ }
services_os_set_exec(op);
-
+
return op;
+
+return_cleanup:
+ free(op->opaque);
+ free(op->rsc);
+ free(op->action);
+ free(op->rclass);
+ free(op->agent);
+ free(op->id);
+ free(op);
+
+ return NULL;
}
svc_action_t *resources_action_create(
const char *name, const char *provider, const char *agent,
const char *action, int interval, int timeout, GHashTable *params)
{
- svc_action_t *op = malloc(sizeof(svc_action_t));
- memset(op, 0, sizeof(svc_action_t));
-
- op->opaque = malloc(sizeof(svc_action_private_t));
- memset(op->opaque, 0, sizeof(svc_action_private_t));
-
- op->rsc = strdup(name);
- op->action = strdup(action);
+ svc_action_t *op;
+
+ if(!(op = calloc(1, sizeof(svc_action_t)))) {
+ return NULL;
+ }
+
+ if(!(op->opaque = calloc(1, sizeof(svc_action_private_t)))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->rsc = strdup(name))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->action = strdup(action))) {
+ goto return_cleanup;
+ }
+
op->interval = interval;
op->timeout = timeout;
- op->rclass = strdup("ocf");
- op->agent = strdup(agent);
- op->provider = strdup(provider);
+ if(!(op->rclass = strdup("ocf"))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->agent = strdup(agent))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->provider = strdup(provider))) {
+ goto return_cleanup;
+ }
op->params = params;
- op->id = malloc(500);
- snprintf(op->id, 500, "%s_%s_%d", name, action, interval);
+ if(asprintf(&op->id, "%s_%s_%d", name, action, interval) == -1) {
+ op->id = NULL;
+ goto return_cleanup;
+ }
- op->opaque->exec = malloc(500);
- snprintf(op->opaque->exec, 500, "%s/resource.d/%s/%s", OCF_ROOT, provider, agent);
+ if(asprintf(&op->opaque->exec, "%s/resource.d/%s/%s", OCF_ROOT,
+ provider, agent) == -1) {
+ op->opaque->exec = NULL;
+ goto return_cleanup;
+ }
+
+ if(!(op->opaque->args[0] = strdup(op->opaque->exec))) {
+ goto return_cleanup;
+ }
+
+ if(!(op->opaque->args[1] = strdup(action))) {
+ goto return_cleanup;
+ }
- op->opaque->args[0] = strdup(op->opaque->exec);
- op->opaque->args[1] = strdup(action);
- op->opaque->args[2] = NULL;
-
return op;
+
+return_cleanup:
+ free(op->rsc);
+ free(op->action);
+ free(op->rclass);
+ free(op->agent);
+ free(op->provider);
+ free(op->id);
+ if(op->opaque) {
+ free(op->opaque->exec);
+ free(op->opaque->args[0]);
+ free(op->opaque->args[1]);
+ }
+ free(op->opaque);
+ free(op);
+
+ return NULL;
}
void services_action_free(svc_action_t *op)
{
if(op == NULL) {
- return;
+ return;
}
-
+
free(op->id);
free(op->opaque->exec);
@@ -118,8 +190,8 @@ void services_action_free(svc_action_t *op)
free(op->stderr_data);
if(op->params) {
- g_hash_table_destroy(op->params);
- op->params = NULL;
+ g_hash_table_destroy(op->params);
+ op->params = NULL;
}
free(op);
@@ -128,42 +200,40 @@ void services_action_free(svc_action_t *op)
gboolean services_action_cancel(const char *name, const char *action, int interval)
{
svc_action_t* op = NULL;
- gboolean found = FALSE;
- char *id = malloc(500);
-
- snprintf(id, 500, "%s_%s_%d", name, action, interval);
- op = g_hash_table_lookup(recurring_actions, id);
- free(id);
-
- if(op) {
- found = TRUE;
- op->opaque->cancel = TRUE;
- mh_debug("Removing %s", op->id);
- if(op->opaque->repeat_timer) {
- g_source_remove(op->opaque->repeat_timer);
- }
- services_action_free(op);
- }
-
- return found;
+ char id[512];
+
+ snprintf(id, sizeof(id), "%s_%s_%d", name, action, interval);
+
+ if (!(op = g_hash_table_lookup(recurring_actions, id))) {
+ return FALSE;
+ }
+
+ op->opaque->cancel = TRUE;
+ mh_debug("Removing %s", op->id);
+ if(op->opaque->repeat_timer) {
+ g_source_remove(op->opaque->repeat_timer);
+ }
+ services_action_free(op);
+
+ return TRUE;
}
gboolean
services_action_async(svc_action_t* op, void (*action_callback)(svc_action_t*))
{
if(action_callback) {
- op->opaque->callback = action_callback;
+ op->opaque->callback = action_callback;
}
if(recurring_actions == NULL) {
- recurring_actions = g_hash_table_new_full(
- g_str_hash, g_str_equal, NULL, NULL);
+ recurring_actions = g_hash_table_new_full(
+ g_str_hash, g_str_equal, NULL, NULL);
}
if(op->interval > 0) {
- g_hash_table_replace(recurring_actions, op->id, op);
+ g_hash_table_replace(recurring_actions, op->id, op);
}
-
+
return services_os_action_execute(op, FALSE);
}
@@ -173,10 +243,10 @@ services_action_sync(svc_action_t* op)
gboolean rc = services_os_action_execute(op, TRUE);
mh_trace(" > %s_%s_%d: %s = %d", op->rsc, op->action, op->interval, op->opaque->exec, op->rc);
if(op->stdout_data) {
- mh_trace(" > stdout: %s", op->stdout_data);
+ mh_trace(" > stdout: %s", op->stdout_data);
}
if(op->stderr_data) {
- mh_trace(" > stderr: %s", op->stderr_data);
+ mh_trace(" > stderr: %s", op->stderr_data);
}
return rc;
}
@@ -187,17 +257,17 @@ get_directory_list(const char *root, gboolean files)
return services_os_get_directory_list(root, files);
}
-GList *services_list(void)
+GList *services_list(void)
{
return services_os_list();
}
-GList *resources_list_ocf_providers(void)
+GList *resources_list_ocf_providers(void)
{
return resources_os_list_ocf_providers();
}
-GList *resources_list_ocf_agents(const char *provider)
+GList *resources_list_ocf_agents(const char *provider)
{
return resources_os_list_ocf_agents(provider);
}
--
1.7.4
13 years
[PATCH] Host agent connection test
by Adam Stokes
---
src/CMakeLists.txt | 2 ++
src/tests/host-agent.t.in | 21 +++++++++++++++++++++
2 files changed, 23 insertions(+), 0 deletions(-)
create mode 100644 src/tests/host-agent.t.in
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b95bbc6..e2dbd5d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -186,4 +186,6 @@ install(TARGETS mcommon mhost msrv DESTINATION lib${LIB_SUFFIX})
install(FILES ${SCHEMAS} DESTINATION share/matahari)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/matahari.h DESTINATION include)
+# tests
+configure_file (${CMAKE_CURRENT_SOURCE_DIR}/tests/host-agent.t.in ${CMAKE_CURRENT_BINARY_DIR}/tests/host-agent.t)
diff --git a/src/tests/host-agent.t.in b/src/tests/host-agent.t.in
new file mode 100644
index 0000000..f671c04
--- /dev/null
+++ b/src/tests/host-agent.t.in
@@ -0,0 +1,21 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+use Test::Simple tests => 1; # Increment as tests are added
+
+=head1
+Matahari host agent test suite
+=cut
+
+my $binary = "matahari-hostd";
+my $broker_host = @MATAHARI_BROKER@;
+my $broker_port = @MATAHARI_PORT@;
+
+my $cmd = "$binary -b $broker_host -p $broker_port";
+`$cmd`;
+
+my @args = ("pidof", $binary);
+ok( $? eq 0, "$binary is running");
+
+`pkill $binary`;
--
1.7.3.3
13 years
Some notes on initial integration of Aeolus and Matahari
by Perry Myers
cross-posting to Aeolus and Matahari upstream projects...
We discussed sort of a pilot project for integration of Matahari into
Aeolus usage. The driver for this is that we need a way for folks
upstream to be able to deploy a single node Aeolus environment managing
1 or more Fedora hosts using kvm/libvirt (or VMware)
The problem is that we don't have a way in this environment to
bootstrap, specifically for Aeolus to get the ip address of the guests
as they are booting on the libvirt/kvm/VMware hosts.
I thought that this would be a good simple first use case for running
Matahari in Aeolus managed guests. So here's what I propose:
* Aeolus uses Image Factory to build images... When those images are
either Fedora, RHEL or Windows IF will include Matahari and puppet in
the core images
* Aeolus will inject basic bootstrapping info for Matahari like broker
ip address and broker auth info (this ignores the more complex post
boot config process Carl has for the time being, this process is
quick and dirty just to get stuff working and then we can expand to
use Carl's process as a second step)
* This injection could happen in a few ways:
- During image build
- During image deployment (using guestfish to inject a config file)
- Other? Could rely on DNS SRV records w/ no broker auth (again this
is meant for developer environments, not production usage)
* When an instance is booted, matahari comes up uses the injected config
and connects to the broker and a "hey I'm a new guest on the bus"
event should be sent out which includes the ip address of the guest
* Simple QMF console written that runs as part of the Conductor
infrastructure that monitors the bus for new 'I'm here' events and
when it sees one it grabs the ip address and stores it in the database
* It then calls a processConfig method (on which Agent?) which gives a
puppet file to a guest agent to process via a call to puppet tool.
Return from that method should be success or failure (with logging
info probably) This means puppet needs to be baked into the image as
well along with matahari.
TODOs:
Matahari Team:
* Write host agent event for 'i'm here configure me, here's my ip
address'
* Write method (which agent?) for processConfig(puppet-conf)
Aeolus Team:
* Need to write a simple QMF Console app that is part of the Conductor
that registers for 'new guest' events and when it sees these reports
that to Conductor and calls the processConfig method with the right
puppet script
* Run a broker alongside the Conductor and other Aeolus core
infrastructure. The ip address of the host running this is what
needs to be pushed into images for bootstrapping
The end goal here should be making it EASY for someone to grab Aeolus
and set it up quickly to manage local resources (i.e. we don't want to
_require_ people playing with Aeolus to need an EC2 account)
Andrew/Adam: Random question, what links all of the top level objects on
a single host together? Is there a common/shared UUID exposed by each
Agent? If a guest comes up and the Host agent appears, I can query that
Host object for UUID. But how do I know which Network Agent is the one
on the same guest as the Host Agent with the UUID that I just got?
Perry
13 years, 1 month
[PATCH 1/5] Make build output slightly less verbose.
by russell@russellbryant.net
This tells gnu make not to print out messages as it enters and leaves
directories.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
GNUmakefile | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/GNUmakefile b/GNUmakefile
index ce16738..8cf41cf 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -37,13 +37,13 @@ linux.build:
@echo "=::=::=::= Setting up for Linux =::=::=::= "
mkdir -p $@
cd $@ && eval "`rpm --eval "%{cmake}" | grep -v -e "^%"`" -DCMAKE_BUILD_TYPE=RelWithDebInfo ..
- @$(MAKE) -C $@
+ @$(MAKE) --no-print-dir -C $@
windows.build:
@echo "=::=::=::= Setting up for Windows =::=::=::= "
mkdir -p $@
cd $@ && eval "`rpm --eval "%{_mingw32_cmake}"`" -DCMAKE_BUILD_TYPE=Release ..
- @$(MAKE) -C $@
+ @$(MAKE) --no-print-dir -C $@
%.check: %.build
DESTDIR=`pwd`/$@ make -C $^ all install
--
1.7.4
13 years, 1 month
[PATCH] src/lib/utilities.c: Fix/simplify part of matahari_uuid()
by russell@russellbryant.net
I first started looking at this particular function because of a
warning emitted by valgrind:
==18466== Conditional jump or move depends on uninitialised value(s)
==18466== at 0x3186A4919E: vfprintf (in /lib64/libc-2.13.90.so)
==18466== by 0x3186A71451: vsnprintf (in /lib64/libc-2.13.90.so)
==18466== by 0x3186A51FE1: snprintf (in /lib64/libc-2.13.90.so)
==18466== by 0x4C20697: host_os_get_uuid (in /usr/lib64/libmhost.so.0.0.1)
==18466== by 0x40CC9D: HostAgent::setup(qpid::management::ManagementAgent*) (in /usr/sbin/matahari-hostd)
==18466== by 0x43A9AA: MatahariAgent::init(int, char**, char const*) (in /usr/sbin/matahari-hostd)
==18466== by 0x40BB9E: main (in /usr/sbin/matahari-hostd)
The changes are:
1) Instead of open coding a loop looking for an end of line or
the end of the buffer, use strchrnul().
2) The old code ignored a possible allocation failure and would
have crashed on snprintf() in that rare case.
3) The old code would also leak memory if the input file had
more than one line in it.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/utilities.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/lib/utilities.c b/src/lib/utilities.c
index cb822fe..01bf26a 100644
--- a/src/lib/utilities.c
+++ b/src/lib/utilities.c
@@ -374,15 +374,9 @@ const char *matahari_uuid(void)
uuid = strdup("unknown");
} else {
- int lpc = 0;
- for (; lpc < data_length; lpc++) {
- switch(buffer[lpc]) {
- case '\0':
- case '\n':
- uuid = malloc(lpc);
- snprintf(uuid, lpc-1, "%s", buffer);
- }
- }
+ char *tmp = strchrnul(buffer, '\n');
+ *tmp = '\0';
+ uuid = strdup(buffer);
}
if(input) {
fclose(input);
--
1.7.4
13 years, 1 month
[PATCH] src/lib/utilities.c: resolve compiler warning
by russell@russellbryant.net
Constify a variable so that the compiler is happy with the usage of
matahari_uuid().
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
CMakeLists.txt | 2 +-
src/lib/utilities.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 19db062..8431e6b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 2.6)
project (MATAHARI)
-set(CMAKE_VERBOSE_MAKEFILE ON)
+set(CMAKE_VERBOSE_MAKEFILE OFF)
set(CPACK_PACKAGE_VERSION_MAJOR "0")
set(CPACK_PACKAGE_VERSION_MINOR "4")
diff --git a/src/lib/utilities.c b/src/lib/utilities.c
index b9d2e0d..cb822fe 100644
--- a/src/lib/utilities.c
+++ b/src/lib/utilities.c
@@ -332,7 +332,7 @@ mh_log_fn(int priority, const char * fmt, ...)
const char *matahari_hostname(void)
{
- static char *hostname = NULL;
+ static const char *hostname = NULL;
if(hostname == NULL) {
sigar_t *sigar;
@@ -343,11 +343,12 @@ const char *matahari_hostname(void)
}
if(hostname != NULL && strcmp(hostname, "localhost") == 0) {
- free(hostname);
+ free((char *) hostname);
hostname = matahari_uuid();
}
mh_trace("Got hostname: %s", hostname);
+
return hostname;
}
--
1.7.4
13 years, 1 month
[PATCH] src/lib/host.c: Resolve memory leaks, minor optimizations
by russell@russellbryant.net
1) Fix memory leaks in host_get_operating_system() and
host_get_architecture(). The code was written as if the local data
buffer was static, but it was not.
2) Minor optimizations - localize variables, only call init() when
the local buffer has not already been populated.
3) One formatting tweak to move a return to its own line.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/host.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/lib/host.c b/src/lib/host.c
index 73e1f6a..cd40db2 100644
--- a/src/lib/host.c
+++ b/src/lib/host.c
@@ -47,7 +47,9 @@ cpuinfo_t cpuinfo = { NULL, 0, 0 };
static void
init(void)
{
- if(host_init.sigar_init) return;
+ if(host_init.sigar_init) {
+ return;
+ }
sigar_open(&host_init.sigar);
host_init.sigar_init = TRUE;
}
@@ -67,14 +69,16 @@ host_get_hostname(void)
const char *
host_get_operating_system(void)
{
- init();
- sigar_sys_info_t sysinfo;
- char *operating_system = NULL;
+ static const char *operating_system = NULL;
if(operating_system == NULL) {
+ sigar_sys_info_t sysinfo;
+
+ init();
sigar_sys_info_get(host_init.sigar, &sysinfo);
operating_system = g_strdup_printf("%s (%s)", sysinfo.vendor_name, sysinfo.version);
}
+
return operating_system;
}
@@ -87,14 +91,16 @@ host_get_cpu_wordsize(void)
const char *
host_get_architecture(void)
{
- init();
- sigar_sys_info_t sysinfo;
- char *arch = NULL;
+ static const char *arch = NULL;
if(arch == NULL) {
+ sigar_sys_info_t sysinfo;
+
+ init();
sigar_sys_info_get(host_init.sigar, &sysinfo);
arch = g_strdup(sysinfo.arch);
}
+
return arch;
}
--
1.7.4
13 years, 1 month
[PATCH] src/lib/host_linux.c: refactor host_os_get_cpu_flags().
by russell@russellbryant.net
This patch refactors a lot of host_get_cpu_flags() and addresses
the following things:
1) Gracefully handle the case where /proc/cpuinfo can't be opened
for some reason.
2) Gracefully handle memory allocation failures.
3) Add a call to pcre_free() so that there is not a memory leak
of the compiled regex.
4) Eliminate unnecessary malloc()/free() calls to improve efficiency.
5) Refactor the loop that looks through the cpuinfo output to take
less code and be more readable (at least to me).
6) Reduce nesting, make formatting more consistent.
7) Localize some variables, make constants static const.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/host_linux.c | 130 ++++++++++++++++++++++++++------------------------
1 files changed, 67 insertions(+), 63 deletions(-)
diff --git a/src/lib/host_linux.c b/src/lib/host_linux.c
index f522562..96e1303 100644
--- a/src/lib/host_linux.c
+++ b/src/lib/host_linux.c
@@ -35,83 +35,89 @@
#include "matahari/host.h"
#include "host_private.h"
-const char *
-host_os_get_cpu_flags(void)
+const char *host_os_get_cpu_flags(void)
{
+ static const char regexstr[] = "(.*\\S)\\s*:\\s*(\\S.*)";
static char *flags = NULL;
-
- size_t chunk = 512;
+
size_t read_chars = 0;
size_t data_length = 0;
-
char *buffer = NULL;
FILE *input = NULL;
+ const char *pcre_error;
+ int pcre_error_offset;
+ pcre *regex = NULL;
+ char *cur, *next;
+
+ if (flags) {
+ return flags;
+ }
- if(flags) {
- return flags;
+ if (!(input = fopen("/proc/cpuinfo", "r"))) {
+ goto done;
}
-
- input = fopen("/proc/cpuinfo", "r");
do {
- buffer = realloc(buffer, chunk + data_length + 1);
- read_chars = fread(buffer + data_length, 1, chunk, input);
- data_length += read_chars;
+ static const size_t chunk = 512;
+
+ buffer = realloc(buffer, chunk + data_length + 1);
+ read_chars = fread(buffer + data_length, 1, chunk, input);
+ data_length += read_chars;
} while (read_chars > 0);
- if(data_length == 0) {
- mh_warn("Could not read from /proc/cpuinfo");
-
- } else {
- const char *regexstr = "(.*\\S)\\s*:\\s*(\\S.*)";
- int expected = 3;
- int found[9];
- const char* pcre_error;
- int pcre_error_offset;
- pcre* regex;
- int offset = 0, lpc = 0, match = 0;
-
- buffer[data_length] = '\0';
- regex = pcre_compile(regexstr, 0, &pcre_error, &pcre_error_offset, NULL);
- if(!regex) {
- mh_err("Unable to compile regular expression '%s' at offset %d: %s",
- regexstr, pcre_error_offset, pcre_error);
- goto done;
- }
-
- for(lpc = 0; lpc < data_length; lpc++) {
-
- switch(buffer[lpc]) {
- case '\n':
- case '\0':
- match = pcre_exec(regex, NULL, buffer+offset, lpc-offset,
- 0, PCRE_NOTEMPTY, found, 9);
-
- if(match == expected) {
- char *name = malloc(2 + found[3] - found[2]);
- snprintf(name, 1 + found[3] - found[2], "%s", buffer + offset + found[2]);
-
- if (strcmp(name, "flags") == 0) {
- flags = malloc(2 + found[5] - found[4]);
- snprintf(flags, 1 + found[5] - found[4], "%s", buffer + offset + found[4]);
- free(name);
- goto done;
- }
- free(name);
- }
- offset = lpc+1;
- }
- }
+ if (data_length == 0) {
+ mh_warn("Could not read from /proc/cpuinfo");
+ goto done;
+ }
+
+ buffer[data_length] = '\0';
+
+ regex = pcre_compile(regexstr, 0, &pcre_error, &pcre_error_offset, NULL);
+ if (!regex) {
+ mh_err("Unable to compile regular expression '%s' at offset %d: %s",
+ regexstr, pcre_error_offset, pcre_error);
+ goto done;
}
-
- done:
- fclose(input);
+
+ next = buffer;
+ while ((cur = strsep(&next, "\n"))) {
+ static const int expected = 3;
+ size_t len;
+ int match;
+ int found[9];
+
+ match = pcre_exec(regex, NULL, cur, strlen(cur),
+ 0, PCRE_NOTEMPTY, found,
+ sizeof(found) / sizeof(found[0]));
+
+ if (match != expected || strncmp(cur + found[2], "flags", 5)) {
+ continue;
+ }
+
+ len = 1 + found[5] - found[4];
+ if (!(flags = malloc(len))) {
+ goto done;
+ }
+ strncpy(flags, cur + found[4], len);
+ flags[len - 1] = '\0';
+ break;
+ }
+
+done:
+ if (input) {
+ fclose(input);
+ }
+
free(buffer);
- if(flags == NULL) {
- flags = strdup("unknown");
+ if (regex) {
+ pcre_free(regex);
}
-
+
+ if (flags == NULL) {
+ flags = strdup("unknown");
+ }
+
return flags;
}
@@ -126,5 +132,3 @@ host_os_shutdown(void)
{
reboot(LINUX_REBOOT_CMD_HALT);
}
-
-
--
1.7.4
13 years, 1 month
[PATCH] MatahariAgent: formatting changes
by russell@russellbryant.net
Strip trailing whitespace. Resolve mixed usage of tabs and spaces
for indentation.
Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
---
src/lib/mh_agent.cpp | 308 ++++++++++++++++++++++++-------------------------
1 files changed, 151 insertions(+), 157 deletions(-)
diff --git a/src/lib/mh_agent.cpp b/src/lib/mh_agent.cpp
index e103dc1..2048b53 100644
--- a/src/lib/mh_agent.cpp
+++ b/src/lib/mh_agent.cpp
@@ -5,12 +5,12 @@
* modify it under the terms of the GNU General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
- *
+ *
* This software 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 library; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
@@ -64,21 +64,21 @@ RegistryRead (HKEY hHive, const wchar_t *szKeyPath, const wchar_t *szValue, char
DWORD nSize = BUFFER_SIZE;
wchar_t szData[BUFFER_SIZE];
long lSuccess = RegOpenKey (hHive, szKeyPath, &hKey);
-
+
if (lSuccess != ERROR_SUCCESS) {
- mh_debug("Could not open %ls key from the registry: %ld", szKeyPath, lSuccess);
- return;
+ mh_debug("Could not open %ls key from the registry: %ld", szKeyPath, lSuccess);
+ return;
}
-
+
lSuccess = RegQueryValueEx (hKey, szValue, NULL, NULL, (LPBYTE) szData, &nSize);
if (lSuccess != ERROR_SUCCESS) {
- mh_debug("Could not read '%ls[%ls]' from the registry: %ld", szKeyPath, szValue, lSuccess);
- return;
+ mh_debug("Could not read '%ls[%ls]' from the registry: %ld", szKeyPath, szValue, lSuccess);
+ return;
}
mh_info("Obtained '%ls[%ls]' = '%ls' from the registry", szKeyPath, szValue, szData);
if(out) {
- *out = (char *)malloc( BUFFER_SIZE );
- wcstombs(*out, szData, (size_t)BUFFER_SIZE);
+ *out = (char *)malloc( BUFFER_SIZE );
+ wcstombs(*out, szData, (size_t)BUFFER_SIZE);
}
}
#else
@@ -109,15 +109,15 @@ print_usage(const char *proc_name)
}
#endif
-static gboolean
+static gboolean
mh_qpid_callback(qmf::AgentSession session, qmf::AgentEvent event, gpointer user_data)
{
MatahariAgent *agent = (MatahariAgent*) user_data;
mh_trace("Qpid message recieved");
if(event.hasDataAddr()) {
- mh_trace("Message is for %s (type: %s)",
- event.getDataAddr().getName().c_str(),
- event.getDataAddr().getAgentName().c_str());
+ mh_trace("Message is for %s (type: %s)",
+ event.getDataAddr().getName().c_str(),
+ event.getDataAddr().getAgentName().c_str());
}
return agent->invoke(session, event, user_data);
}
@@ -147,159 +147,153 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
int serverport = MATAHARI_PORT;
/* Set up basic logging */
- mh_log_init(proc_name, LOG_INFO, FALSE);
+ mh_log_init(proc_name, LOG_INFO, FALSE);
#ifdef WIN32
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"DebugLevel",
- &value);
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"DebugLevel",
+ &value);
if(value) {
- mh_log_level = LOG_INFO+atoi(value);
- free(value);
- value = NULL;
+ mh_log_level = LOG_INFO+atoi(value);
+ free(value);
+ value = NULL;
}
-
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"Broker",
- &value);
+
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"Broker",
+ &value);
if(value) {
- servername = value;
- free(value);
- value = NULL;
+ servername = value;
+ free(value);
+ value = NULL;
}
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"Port",
- &value);
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"Port",
+ &value);
if(value) {
- serverport = atoi(value);
- free(value);
- value = NULL;
+ serverport = atoi(value);
+ free(value);
+ value = NULL;
}
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"Service",
- &value);
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"Service",
+ &value);
if(value) {
- service = value;
- free(value);
- value = NULL;
+ service = value;
+ free(value);
+ value = NULL;
}
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"User",
- &value);
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"User",
+ &value);
if(value) {
username = value;
- free(value);
- value = NULL;
+ free(value);
+ value = NULL;
}
- RegistryRead (
- HKEY_LOCAL_MACHINE,
- L"SYSTEM\\CurrentControlSet\\services\\Matahari",
- L"Password",
- &value);
+ RegistryRead(HKEY_LOCAL_MACHINE,
+ L"SYSTEM\\CurrentControlSet\\services\\Matahari",
+ L"Password",
+ &value);
if(value) {
password = value;
- free(value);
- value = NULL;
+ free(value);
+ value = NULL;
}
#else
-
+
// Get args
while ((arg = getopt_long(argc, argv, "hdb:gu:P:s:p:v", opt, &idx)) != -1) {
- switch (arg) {
- case 'h':
- case '?':
- print_usage(proc_name);
- exit(0);
- break;
- case 'd':
- daemonize = true;
- break;
- case 'v':
- mh_log_level++;
- mh_enable_stderr(1);
- break;
- case 's':
- if (optarg) {
- service = optarg;
- } else {
- print_usage(proc_name);
- exit(1);
- }
- break;
- case 'u':
- if (optarg) {
- username = optarg;
- } else {
- print_usage(proc_name);
- exit(1);
- }
- break;
- case 'P':
- if (optarg) {
- password = optarg;
- } else {
- print_usage(proc_name);
- exit(1);
- }
- break;
- case 'g':
- gssapi = true;
- break;
- case 'p':
- if (optarg) {
- serverport = atoi(optarg);
- } else {
- print_usage(proc_name);
- exit(1);
- }
- break;
- case 'b':
- if (optarg) {
- servername = optarg;
- } else {
- print_usage(proc_name);
- exit(1);
- }
- break;
- default:
- fprintf(stderr, "unsupported option '-%c'. See --help.\n", arg);
- print_usage(proc_name);
- exit(0);
- break;
- }
+ switch (arg) {
+ case 'h':
+ case '?':
+ print_usage(proc_name);
+ exit(0);
+ break;
+ case 'd':
+ daemonize = true;
+ break;
+ case 'v':
+ mh_log_level++;
+ mh_enable_stderr(1);
+ break;
+ case 's':
+ if (optarg) {
+ service = optarg;
+ } else {
+ print_usage(proc_name);
+ exit(1);
+ }
+ break;
+ case 'u':
+ if (optarg) {
+ username = optarg;
+ } else {
+ print_usage(proc_name);
+ exit(1);
+ }
+ break;
+ case 'P':
+ if (optarg) {
+ password = optarg;
+ } else {
+ print_usage(proc_name);
+ exit(1);
+ }
+ break;
+ case 'g':
+ gssapi = true;
+ break;
+ case 'p':
+ if (optarg) {
+ serverport = atoi(optarg);
+ } else {
+ print_usage(proc_name);
+ exit(1);
+ }
+ break;
+ case 'b':
+ if (optarg) {
+ servername = optarg;
+ } else {
+ print_usage(proc_name);
+ exit(1);
+ }
+ break;
+ default:
+ fprintf(stderr, "unsupported option '-%c'. See --help.\n", arg);
+ print_usage(proc_name);
+ exit(0);
+ break;
+ }
}
if (daemonize == true) {
- if (daemon(0, 0) < 0) {
- fprintf(stderr, "Error daemonizing: %s\n", strerror(errno));
- exit(1);
- }
+ if (daemon(0, 0) < 0) {
+ fprintf(stderr, "Error daemonizing: %s\n", strerror(errno));
+ exit(1);
+ }
}
#endif
if (servername.length() == 0) {
- servername = MATAHARI_BROKER;
+ servername = MATAHARI_BROKER;
}
/* Re-initialize logging now that we've completed option processing */
@@ -312,16 +306,16 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
qpid::types::Variant::Map options;
options["reconnect"] = bool(true);
if (!username.empty()) {
- options["username"] = username.c_str();
+ options["username"] = username.c_str();
}
if (!password.empty()) {
- options["password"] = password.c_str();
+ options["password"] = password.c_str();
}
if (!service.empty()) {
- options["sasl-service"] = service.c_str();
+ options["sasl-service"] = service.c_str();
}
if (gssapi) {
- options["sasl-mechanism"] = "GSSAPI";
+ options["sasl-mechanism"] = "GSSAPI";
}
std::stringstream url;
@@ -341,14 +335,14 @@ MatahariAgent::init(int argc, char **argv, const char* proc_name)
/* Do any setup required by our agent */
if(this->setup(_agent_session) < 0) {
- mh_err("Failed to set up broker connection to %s for %s\n",
- url.str().c_str(), proc_name);
- return -1;
+ mh_err("Failed to set up broker connection to %s for %s\n",
+ url.str().c_str(), proc_name);
+ return -1;
}
this->mainloop = g_main_new(FALSE);
- this->qpid_source = mainloop_add_qmf(
- G_PRIORITY_HIGH, _agent_session, mh_qpid_callback, mh_qpid_disconnect, this);
+ this->qpid_source = mainloop_add_qmf(G_PRIORITY_HIGH, _agent_session,
+ mh_qpid_callback, mh_qpid_disconnect, this);
return 0;
}
@@ -365,9 +359,9 @@ mainloop_qmf_prepare(GSource* source, gint *timeout)
{
mainloop_qmf_t *qmf = (mainloop_qmf_t*)source;
if (qmf->event) {
- return TRUE;
+ return TRUE;
}
-
+
*timeout = 1;
return FALSE;
}
@@ -377,10 +371,9 @@ mainloop_qmf_check(GSource* source)
{
mainloop_qmf_t *qmf = (mainloop_qmf_t*)source;
if (qmf->event) {
- return TRUE;
-
+ return TRUE;
} else if(qmf->session.nextEvent(qmf->event, qpid::messaging::Duration::IMMEDIATE)) {
- return TRUE;
+ return TRUE;
}
return FALSE;
}
@@ -391,15 +384,15 @@ mainloop_qmf_dispatch(GSource *source, GSourceFunc callback, gpointer userdata)
mainloop_qmf_t *qmf = (mainloop_qmf_t*)source;
mh_trace("%p", source);
if (qmf->dispatch != NULL) {
- qmf::AgentEvent event = qmf->event;
- qmf->event = NULL;
-
- if(qmf->dispatch(qmf->session, event, qmf->user_data) == FALSE) {
- g_source_unref(source); /* Really? */
- return FALSE;
- }
+ qmf::AgentEvent event = qmf->event;
+ qmf->event = NULL;
+
+ if(qmf->dispatch(qmf->session, event, qmf->user_data) == FALSE) {
+ g_source_unref(source); /* Really? */
+ return FALSE;
+ }
}
-
+
return TRUE;
}
@@ -410,7 +403,7 @@ mainloop_qmf_destroy(GSource* source)
mh_trace("%p", source);
if (qmf->dnotify) {
- qmf->dnotify(qmf->user_data);
+ qmf->dnotify(qmf->user_data);
}
}
@@ -423,8 +416,9 @@ static GSourceFuncs mainloop_qmf_funcs = {
mainloop_qmf_t*
mainloop_add_qmf(int priority, qmf::AgentSession session,
- gboolean (*dispatch)(qmf::AgentSession session, qmf::AgentEvent event, gpointer userdata),
- GDestroyNotify notify, gpointer userdata)
+ gboolean (*dispatch)(qmf::AgentSession session,
+ qmf::AgentEvent event, gpointer userdata),
+ GDestroyNotify notify, gpointer userdata)
{
GSource *source = NULL;
mainloop_qmf_t *qmf_source = NULL;
@@ -447,7 +441,7 @@ mainloop_add_qmf(int priority, qmf::AgentSession session,
g_source_set_priority(source, priority);
g_source_set_can_recurse(source, FALSE);
-
+
qmf_source->id = g_source_attach(source, NULL);
mh_info("Added source: %d", qmf_source->id);
return qmf_source;
@@ -459,6 +453,6 @@ mainloop_destroy_qmf(mainloop_qmf_t* source)
g_source_remove(source->id);
source->id = 0;
g_source_unref((GSource*)source);
-
+
return TRUE;
}
--
1.7.4
13 years, 1 month