From f6151c55e2af06fe2204bcab62cc19e33964e5ce Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Wed, 5 Aug 2015 14:57:36 -0400 Subject: [PATCH] Send data pending status as part of payload instead of an invalid error MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This ensures that the session daemon's logs don't contain false positives every time a data pending command is replied-to. The use of invalid error codes 0 and 1 causes the log to contain misleading entries of the form: Sending response (size: 16, retcode: Unknown error code (0)) [...] This commit also proves that it is possible, contrary to what the previous comment indicated, to send a boolean value over a socket without using undocumented error codes. Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/main.c | 30 +++++++++++++++++++++++++++++- src/lib/lttng-ctl/lttng-ctl.c | 21 +++++++++++---------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index fa5cb901e..792f519ab 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -3766,7 +3766,35 @@ skip_domain: } case LTTNG_DATA_PENDING: { - ret = cmd_data_pending(cmd_ctx->session); + int pending_ret; + + /* 1 byte to return whether or not data is pending */ + ret = setup_lttng_msg(cmd_ctx, 1); + if (ret < 0) { + goto setup_error; + } + + pending_ret = cmd_data_pending(cmd_ctx->session); + /* + * FIXME + * + * This function may returns 0 or 1 to indicate whether or not + * there is data pending. In case of error, it should return an + * LTTNG_ERR code. However, some code paths may still return + * a nondescript error code, which we handle by returning an + * "unknown" error. + */ + if (pending_ret == 0 || pending_ret == 1) { + ret = LTTNG_OK; + } else if (pending_ret < 0) { + ret = LTTNG_ERR_UNK; + goto setup_error; + } else { + ret = pending_ret; + goto setup_error; + } + + *cmd_ctx->llm->payload = (uint8_t) pending_ret; break; } case LTTNG_SNAPSHOT_ADD_OUTPUT: diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index 42c9d1390..5f078c94a 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -1880,6 +1880,7 @@ int lttng_data_pending(const char *session_name) { int ret; struct lttcomm_session_msg lsm; + uint8_t *pending = NULL; if (session_name == NULL) { return -LTTNG_ERR_INVALID; @@ -1891,18 +1892,18 @@ int lttng_data_pending(const char *session_name) lttng_ctl_copy_string(lsm.session.name, session_name, sizeof(lsm.session.name)); - ret = lttng_ctl_ask_sessiond(&lsm, NULL); - - /* - * The lttng_ctl_ask_sessiond function negate the return code if it's not - * LTTNG_OK so getting -1 means that the reply ret_code was 1 thus meaning - * that the data is available. Yes it is hackish but for now this is the - * only way. - */ - if (ret == -1) { - ret = 1; + ret = lttng_ctl_ask_sessiond(&lsm, (void **) &pending); + if (ret < 0) { + goto end; + } else if (ret != 1) { + /* Unexpected payload size */ + ret = -LTTNG_ERR_INVALID; + goto end; } + ret = (int) *pending; +end: + free(pending); return ret; } -- 2.34.1