The lttng/commands/create.c patch improves the usage() printout and documents
and enforces the return values for the cmd_create chain of calls.
The lib/lttng-ctl/lttng-ctl.c patch (the major part of this submission)
documents the return values of all functions. Some points:
* lttng_list_tracepoints()'s documentation was wrong.
* check_tracing_group() had grp_id = getgroups(...); if (grp_id < -1) { ...
but, according to the getgroups() documentation, the return value can never be
< -1
* the snprintf() calls of set_session_daemon_path() cannot return < 0 since GNU
C 2.1; the ifs were rewritten to handle either case
* ask_sessiond() still does not compare the number of bytes received with the
number requested. What should it do if they don't match? Throw everything
away and report an error?
* set_session_daemon_path() was rewritten to reduce redundant segments of code
------------------------------
>From b200f661f1b17c6fb63147e0ad9304cf462b84e6 Fri, 27 Jan 2012 13:29:29 -0500
From: Daniel U. Thibault <[email protected]>
Date: Fri, 27 Jan 2012 13:29:14 -0500
Subject: [PATCH] lttng-tools lib/lttng-ctl/lttng-ctl.c : Document return values
and more
diff --git a/src/bin/lttng/commands/create.c b/src/bin/lttng/commands/create.c
index 34754c7..94ff634 100644
--- a/src/bin/lttng/commands/create.c
+++ b/src/bin/lttng/commands/create.c
@@ -52,8 +52,9 @@
{
fprintf(ofp, "usage: lttng create [options] [NAME]\n");
fprintf(ofp, "\n");
+ fprintf(ofp, " The default NAME is 'auto-yyyymmdd-hhmmss'\n");
fprintf(ofp, " -h, --help Show this help\n");
- fprintf(ofp, " --list-options Simple listing of options\n");
+ fprintf(ofp, " --list-options Simple listing of options\n");
fprintf(ofp, " -o, --output PATH Specify output path for traces\n");
fprintf(ofp, "\n");
}
@@ -61,12 +62,13 @@
/*
* create_session
*
- * Create a tracing session. If no name specified, a default name will be
- * generated.
+ * Create a tracing session.
+ * If no name is specified, a default name is generated.
+ * Returns one of the CMD_* result constants.
*/
static int create_session()
{
- int ret, have_name = 0;
+ int ret;
char datetime[16];
char *session_name, *traces_path = NULL, *alloc_path = NULL;
time_t rawtime;
@@ -79,37 +81,33 @@
/* Auto session name creation */
if (opt_session_name == NULL) {
- ret = asprintf(&session_name, "auto-%s", datetime);
+ ret = asprintf(&session_name, "auto");
if (ret < 0) {
perror("asprintf session name");
+ ret = CMD_ERROR;
goto error;
}
DBG("Auto session name set to %s", session_name);
} else {
session_name = opt_session_name;
- have_name = 1;
}
/* Auto output path */
if (opt_output_path == NULL) {
alloc_path = strdup(config_get_default_path());
if (alloc_path == NULL) {
- ERR("Home path not found.\n \
- Please specify an output path using -o,
--output PATH");
+ ERR("Home path not found.\n%s"
+ "Please specify an output path using -o,
--output PATH\n");
ret = CMD_FATAL;
goto error;
}
- if (have_name) {
- ret = asprintf(&traces_path, "%s/"
DEFAULT_TRACE_DIR_NAME
+ ret = asprintf(&traces_path, "%s/" DEFAULT_TRACE_DIR_NAME
"/%s-%s", alloc_path, session_name,
datetime);
- } else {
- ret = asprintf(&traces_path, "%s/"
DEFAULT_TRACE_DIR_NAME
- "/%s", alloc_path, session_name);
- }
if (ret < 0) {
perror("asprintf trace dir name");
+ ret = CMD_ERROR;
goto error;
}
} else {
@@ -150,6 +148,7 @@
* cmd_create
*
* The 'create <options>' first level command
+ * Returns one of the CMD_* result constants.
*/
int cmd_create(int argc, const char **argv)
{
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index ebb76ec..aeef7f0 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -83,8 +83,8 @@
/*
* Send lttcomm_session_msg to the session daemon.
*
- * On success, return 0
- * On error, return error code
+ * On success, returns the number of bytes sent (>=0)
+ * On error, returns -1
*/
static int send_session_msg(struct lttcomm_session_msg *lsm)
{
@@ -105,8 +105,8 @@
/*
* Receive data from the sessiond socket.
*
- * On success, return 0
- * On error, return recv() error code
+ * On success, returns the number of bytes received (>=0)
+ * On error, returns -1 (recvmsg() error) or -ENOTCONN
*/
static int recv_data_sessiond(void *buf, size_t len)
{
@@ -124,7 +124,7 @@
}
/*
- * Check if the specified group name exist.
+ * Check if we are in the specified group.
*
* If yes return 1, else return -1.
*/
@@ -153,13 +153,15 @@
}
/* Alloc group list of the right size */
+ /* Note that malloc(0) will return a valid pointer */
grp_list = malloc(grp_list_size * sizeof(gid_t));
if (!grp_list) {
- ret = -1;
+ perror("malloc");
goto end;
}
+
grp_id = getgroups(grp_list_size, grp_list);
- if (grp_id < -1) {
+ if (grp_id < 0) {
perror("getgroups");
goto free_list;
}
@@ -209,8 +211,10 @@
}
/*
- * Set sessiond socket path by putting it in the global sessiond_sock_path
- * variable.
+ * Set sessiond socket path by putting it in the global
+ * sessiond_sock_path variable.
+ * Returns 0 on success,
+ * -ENOMEM on failure (the sessiond socket path is somehow too long)
*/
static int set_session_daemon_path(void)
{
@@ -225,35 +229,31 @@
in_tgroup = check_tracing_group(tracing_group);
}
- if (uid == 0) {
- /* Root */
+ if ((uid == 0) || in_tgroup) {
copy_string(sessiond_sock_path,
DEFAULT_GLOBAL_CLIENT_UNIX_SOCK,
sizeof(sessiond_sock_path));
- } else if (in_tgroup) {
- /* Tracing group */
- copy_string(sessiond_sock_path,
- DEFAULT_GLOBAL_CLIENT_UNIX_SOCK,
- sizeof(sessiond_sock_path));
-
- ret = try_connect_sessiond(sessiond_sock_path);
- if (ret < 0) {
- /* Global session daemon not available */
- if (snprintf(sessiond_sock_path,
sizeof(sessiond_sock_path),
- DEFAULT_HOME_CLIENT_UNIX_SOCK,
- getenv("HOME")) < 0) {
- return -ENOMEM;
- }
+ }
+ if (uid != 0) {
+ if (in_tgroup) {
+ /* Tracing group */
+ ret = try_connect_sessiond(sessiond_sock_path);
+ if (ret >= 0) goto end;
+ /* Global session daemon not available... */
}
- } else {
- /* Not in tracing group and not root, default */
- if (snprintf(sessiond_sock_path, PATH_MAX,
- DEFAULT_HOME_CLIENT_UNIX_SOCK,
- getenv("HOME")) < 0) {
+ /* ...or not in tracing group (and not root), default */
+ /*
+ * With GNU C < 2.1, snprintf returns -1 if the target buffer
is too small;
+ * With GNU C >= 2.1, snprintf returns the required size
(excluding closing null)
+ */
+ ret = snprintf(sessiond_sock_path, sizeof(sessiond_sock_path),
+ DEFAULT_HOME_CLIENT_UNIX_SOCK,
+ getenv("HOME"));
+ if ((ret < 0) || (ret >= sizeof(sessiond_sock_path))) {
return -ENOMEM;
}
}
-
+end:
return 0;
}
@@ -268,7 +268,7 @@
ret = set_session_daemon_path();
if (ret < 0) {
- return ret;
+ return -1; /* set_session_daemon_path() returns -ENOMEM */
}
/* Connect to the sesssion daemon */
@@ -284,7 +284,8 @@
}
/*
- * Clean disconnect the session daemon.
+ * Clean disconnect from the session daemon.
+ * On success, return 0. On error, return -1.
*/
static int disconnect_sessiond(void)
{
@@ -373,6 +374,7 @@
/*
* Create lttng handle and return pointer.
+ * The returned pointer will be NULL in case of malloc() error.
*/
struct lttng_handle *lttng_create_handle(const char *session_name,
struct lttng_domain *domain)
@@ -408,6 +410,7 @@
/*
* Register an outside consumer.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_register_consumer(struct lttng_handle *handle,
const char *socket_path)
@@ -425,7 +428,8 @@
}
/*
- * Start tracing for all trace of the session.
+ * Start tracing for all traces of the session.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_start_tracing(const char *session_name)
{
@@ -443,7 +447,8 @@
}
/*
- * Stop tracing for all trace of the session.
+ * Stop tracing for all traces of the session.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_stop_tracing(const char *session_name)
{
@@ -495,7 +500,10 @@
}
/*
- * Enable event
+ * Enable event(s) for a channel.
+ * If no event name is specified, all events are enabled.
+ * If no channel name is specified, the default 'channel0' is used.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_enable_event(struct lttng_handle *handle,
struct lttng_event *ev, const char *channel_name)
@@ -507,13 +515,9 @@
}
/* If no channel name, we put the default name */
- if (channel_name == NULL) {
- copy_string(lsm.u.enable.channel_name, DEFAULT_CHANNEL_NAME,
- sizeof(lsm.u.enable.channel_name));
- } else {
- copy_string(lsm.u.enable.channel_name, channel_name,
- sizeof(lsm.u.enable.channel_name));
- }
+ copy_string(lsm.u.enable.channel_name,
+ channel_name ? channel_name : DEFAULT_CHANNEL_NAME,
+ sizeof(lsm.u.enable.channel_name));
copy_lttng_domain(&lsm.domain, &handle->domain);
@@ -531,7 +535,10 @@
}
/*
- * Disable event of a channel and domain.
+ * Disable event(s) of a channel and domain.
+ * If no event name is specified, all events are disabled.
+ * If no channel name is specified, the default 'channel0' is used.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_disable_event(struct lttng_handle *handle, const char *name,
const char *channel_name)
@@ -542,13 +549,9 @@
return -1;
}
- if (channel_name) {
- copy_string(lsm.u.disable.channel_name, channel_name,
- sizeof(lsm.u.disable.channel_name));
- } else {
- copy_string(lsm.u.disable.channel_name, DEFAULT_CHANNEL_NAME,
- sizeof(lsm.u.disable.channel_name));
- }
+ copy_string(lsm.u.disable.channel_name,
+ channel_name ? channel_name : DEFAULT_CHANNEL_NAME,
+ sizeof(lsm.u.disable.channel_name));
copy_lttng_domain(&lsm.domain, &handle->domain);
@@ -566,7 +569,8 @@
}
/*
- * Enable channel per domain
+ * Enable channel per domain
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_enable_channel(struct lttng_handle *handle,
struct lttng_channel *chan)
@@ -593,7 +597,8 @@
}
/*
- * All tracing will be stopped for registered events of the channel.
+ * All tracing will be stopped for registered events of the channel.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_disable_channel(struct lttng_handle *handle, const char *name)
{
@@ -618,10 +623,10 @@
}
/*
- * List all available tracepoints of domain.
- *
- * Return the size (bytes) of the list and set the events array.
- * On error, return negative value.
+ * Lists all available tracepoints of domain.
+ * Sets the contents of the events array.
+ * Returns the number of lttng_event entries in events;
+ * on error, returns a negative value.
*/
int lttng_list_tracepoints(struct lttng_handle *handle,
struct lttng_event **events)
@@ -645,10 +650,12 @@
}
/*
- * Return a human readable string of code
+ * Returns a human readable string describing
+ * the error code (a negative value).
*/
const char *lttng_strerror(int code)
{
+ /* lttcomm error codes range from -LTTCOMM_OK down to -LTTCOMM_NR */
if (code > -LTTCOMM_OK) {
return "Ended with errors";
}
@@ -657,7 +664,8 @@
}
/*
- * Create a brand new session using name.
+ * Create a brand new session using name and path.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_create_session(const char *name, const char *path)
{
@@ -672,6 +680,7 @@
/*
* Destroy session using name.
+ * Returns size of returned session payload data or a negative error code.
*/
int lttng_destroy_session(const char *session_name)
{
@@ -690,9 +699,9 @@
/*
* Ask the session daemon for all available sessions.
- *
- * Return number of session.
- * On error, return negative value.
+ * Sets the contents of the sessions array.
+ * Returns the number of lttng_session entries in sessions;
+ * on error, returns a negative value.
*/
int lttng_list_sessions(struct lttng_session **sessions)
{
@@ -709,7 +718,10 @@
}
/*
- * List domain of a session.
+ * Ask the session daemon for all available domains of a session.
+ * Sets the contents of the domains array.
+ * Returns the number of lttng_domain entries in domains;
+ * on error, returns a negative value.
*/
int lttng_list_domains(const char *session_name,
struct lttng_domain **domains)
@@ -734,7 +746,10 @@
}
/*
- * List channels of a session
+ * Ask the session daemon for all available channels of a session.
+ * Sets the contents of the channels array.
+ * Returns the number of lttng_channel entries in channels;
+ * on error, returns a negative value.
*/
int lttng_list_channels(struct lttng_handle *handle,
struct lttng_channel **channels)
@@ -761,7 +776,10 @@
}
/*
- * List events of a session channel.
+ * Ask the session daemon for all available events of a session channel.
+ * Sets the contents of the events array.
+ * Returns the number of lttng_event entries in events;
+ * on error, returns a negative value.
*/
int lttng_list_events(struct lttng_handle *handle,
const char *channel_name, struct lttng_event **events)
@@ -791,8 +809,9 @@
}
/*
- * Set tracing group variable with name. This function allocate memory pointed
- * by tracing_group.
+ * Sets the tracing_group variable with name.
+ * This function allocates memory pointed to by tracing_group.
+ * On success, returns 0, on error, returns -1 (null name) or -ENOMEM.
*/
int lttng_set_tracing_group(const char *name)
{
@@ -831,6 +850,7 @@
/*
* Set default channel attributes.
+ * If either or both of the arguments are null, nothing happens.
*/
void lttng_channel_set_default_attr(struct lttng_domain *domain,
struct lttng_channel_attr *attr)
@@ -875,7 +895,7 @@
* Check if session daemon is alive.
*
* Return 1 if alive or 0 if not.
- * On error return -1
+ * On error returns a negative value.
*/
int lttng_session_daemon_alive(void)
{
------------------------------
Daniel U. Thibault
R & D pour la défense Canada - Valcartier (RDDC Valcartier) / Defence R&D
Canada - Valcartier (DRDC Valcartier)
Système de systèmes (SdS) / System of Systems (SoS)
Solutions informatiques et expérimentations (SIE) / Computing Solutions and
Experimentations (CSE)
2459 Boul. Pie XI Nord
Québec, QC G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC: 918V QSDJ
Gouvernement du Canada / Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>
_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
