From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel.Thibault@drdc-rddc.gc.ca (Thibault, Daniel) Date: Fri, 27 Jan 2012 13:41:07 -0500 Subject: [lttng-dev] [PATCH] lttng-tools lib/lttng-ctl/lttng-ctl.c : Document return values and more In-Reply-To: References: Message-ID: <3CA1A65C03C1A34C81BBC6272782E69680895F@valcartierex01.valcartier.drdc-rddc.gc.ca> 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 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 ' 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