[lttng-dev] src/bin/lttng/commands/snapshot.c

[Thibault, Daniel]

   In LTTng 2.3's src/bin/lttng/commands/snapshot.c, we have:

static int add_output(const char *url)
{
	int ret;
	struct lttng_snapshot_output *output = NULL;

	if (!url && (!opt_data_url || !opt_ctrl_url)) {
		ret = CMD_ERROR;
		goto error;
	}
[...]
}

static int cmd_add_output(int argc, const char **argv)
{
	int ret = CMD_SUCCESS;

	if (argc < 2 && (!opt_data_url || !opt_ctrl_url)) {
		usage(stderr);
		ret = CMD_ERROR;
		goto end;
	}
	ret = add_output(argv[1]);
end:
	return ret;
}

add_output() is only called from cmd_add_output(), and the latter's argument validation (combined with the validation done by handle_command()) guarantees that add_output()'s url is NULL only when opt_data_url and opt_ctrl_url are both non-NULL.  The initial validation add_output() conducts is thus unnecessary (that if will always fail).

Another potential problem is cmd_record():

static int cmd_record(int argc, const char **argv)
{
	int ret;

	if (argc == 2) {
		/* With a given URL */
		ret = record(argv[1]);
	} else {
		ret = record(NULL);
	}
	return ret;
}

The if (argc == 2) test is unfortunate, because it means 'lttng snapshot record url1 url2' will result in record(NULL), an unexpected result from any user's point of view.  The test should be if (argc >= 2), which is what cmd_add_output and cmd_del_output both do.

Daniel U. Thibault
Protection des systèmes et contremesures (PSC) | Systems Protection & Countermeasures (SPC)
Cyber sécurité pour les missions essentielles (CME) | Mission Critical Cyber Security (MCCS)
R & D pour la défense Canada - Valcartier (RDDC Valcartier) | Defence R&D Canada - Valcartier (DRDC Valcartier)
2459 route de la Bravoure
Québec QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC : 918V QSDJ <http://www.travelgis.com/map.asp?addr=918V%20QSDJ>
Gouvernement du Canada | Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>