[ltt-dev] [UST PATCH] Introduce a new communication protocol for UST

David Goulet david.goulet at polymtl.ca
Wed Oct 13 15:01:58 EDT 2010


Comments below :

On 10-10-12 03:59 AM, Nils Carlson wrote:
> This is once again a bit of a code-dump. The principal of this
> patch is to get rid of most string parsing in UST and most
> dynamic heap memory allocation by libust. A secondary goal
> has been to introduce a command-response model so all commands
> receive responses with a result code. This has been achived
> through the following steps.
>
>   1. Create standardised message containers as structs ending in
>      a char array. The char array is used to pack strings into
>      while pointers in the struct point positions relative the data
>      segment in the struct. Unpacking the struct upon reception is
>      a simple matter of adding to the pointers the position in memory
>      of the char array.
>
> 2.  Keeping a char array permanently allocated in libust that is
>      used to receive incoming data, avoiding allocation. This array
>      is large enough to receive any of the pre-defined message
>      structs.
>
> 3.  Replacing all string matching for commands with enum based switch
>      statements. This will scale better over time.
>
> 4.  All commands now receive responses containing a result as a negative
>      errno. Libustcmd now returns -1 and sets errno according to these.
>      Eventually these will need to be documented in manpages with
>      what each errno implies. Ustctl needs to check these in turn and
>      give meaningfull feedback.
>
> 5.  Cleaning up tracectl.c quite a bit separating out control functions
>      and message handling.
>
> 6.  Move channel marker scanning from tracectl out to ustctl.
>
> David, I hope you can stomach this patch. But once it's in I hope
> most following patches will be much smaller, until we have to re-write
> libustd that is.... :-)
>
> Signed-off-by: Nils Carlson<nils.carlson at ericsson.com>
> ---
>   include/ust/ustcmd.h |   32 +-
>   include/ust/ustd.h   |    5 +-
>   libust/tracectl.c    | 1313 +++++++++++++++++++++++---------------------------
>   libustcmd/ustcmd.c   |  431 ++++++++++-------
>   libustcomm/ustcomm.c |  411 +++++++++-------
>   libustcomm/ustcomm.h |  141 +++++-
>   libustd/libustd.c    |  595 +++++++++++++----------
>   ustctl/ustctl.c      |   99 ++++-
>   8 files changed, 1649 insertions(+), 1378 deletions(-)
>
> diff --git a/include/ust/ustcmd.h b/include/ust/ustcmd.h
> index 986ae61..b32fe36 100644
> --- a/include/ust/ustcmd.h
> +++ b/include/ust/ustcmd.h
> @@ -48,25 +48,27 @@ struct trace_event_status {
>   };
>
>   extern pid_t *ustcmd_get_online_pids(void);
> -extern int ustcmd_set_marker_state(const char *, int, pid_t);
> -extern int ustcmd_set_subbuf_size(const char *, pid_t);
> -extern int ustcmd_set_subbuf_num(const char *, pid_t);
> -extern int ustcmd_get_subbuf_size(const char *, pid_t);
> -extern int ustcmd_get_subbuf_num(const char *, pid_t);
> -extern int ustcmd_destroy_trace(pid_t);
> -extern int ustcmd_setup_and_start(pid_t);
> -extern int ustcmd_stop_trace(pid_t);
> -extern int ustcmd_create_trace(pid_t);
> -extern int ustcmd_start_trace(pid_t);
> -extern int ustcmd_alloc_trace(pid_t);
> +extern int ustcmd_set_marker_state(const char *channel, const char *marker,
> +int state, pid_t pid);
> +extern int ustcmd_set_subbuf_size(const char *channel, unsigned int subbuf_size,
> +				  pid_t pid);
> +extern int ustcmd_set_subbuf_num(const char *channel, unsigned int num,
> +				 pid_t pid);
> +extern int ustcmd_get_subbuf_size(const char *channel, pid_t pid);
> +extern int ustcmd_get_subbuf_num(const char *channel, pid_t pid);
> +extern int ustcmd_destroy_trace(pid_t pid);
> +extern int ustcmd_setup_and_start(pid_t pid);
> +extern int ustcmd_stop_trace(pid_t pid);
> +extern int ustcmd_create_trace(pid_t pid);
> +extern int ustcmd_start_trace(pid_t pid);
> +extern int ustcmd_alloc_trace(pid_t pid);
>   extern int ustcmd_free_cmsf(struct marker_status *);
>   extern unsigned int ustcmd_count_nl(const char *);
> -extern int ustcmd_send_cmd(const char *, pid_t, char **);
>   extern int ustcmd_get_cmsf(struct marker_status **, pid_t);
>   extern int ustcmd_free_tes(struct trace_event_status *);
>   extern int ustcmd_get_tes(struct trace_event_status **, pid_t);
> -extern int ustcmd_set_sock_path(const char *, pid_t);
> -extern int ustcmd_get_sock_path(char **, pid_t);
> -extern int ustcmd_force_switch(pid_t);
> +extern int ustcmd_set_sock_path(const char *sock_path, pid_t pid);
> +extern int ustcmd_get_sock_path(char **sock_path, pid_t pid);
> +extern int ustcmd_force_switch(pid_t pid);
>
>   #endif /* _USTCMD_H */
> diff --git a/include/ust/ustd.h b/include/ust/ustd.h
> index 7ce063f..01b88d7 100644
> --- a/include/ust/ustd.h
> +++ b/include/ust/ustd.h
> @@ -36,7 +36,10 @@
>   struct ustcomm_sock;
>
>   struct buffer_info {
> -	const char *name;
> +	char *name;
> +	char *channel;
> +	int channel_cpu;
> +
>   	pid_t pid;
>   	int app_sock;
>   	/* The pipe file descriptor */
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index 2e94365..46d69c2 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -45,17 +45,16 @@
>   #include "buffers.h"
>   #include "marker-control.h"
>
> -#define USTSIGNAL SIGIO
> -
> -#define MAX_MSG_SIZE (100)
> -#define MSG_NOTIF 1
> -#define MSG_REGISTER_NOTIF 2
> -
>   /* This should only be accessed by the constructor, before the creation
>    * of the listener, and then only by the listener.
>    */
>   s64 pidunique = -1LL;
>
> +static struct ustcomm_header _receive_header;
> +static struct ustcomm_header *receive_header =&_receive_header;

Just tell me why you do this little trick here and across the code?

> +static char receive_buffer[USTCOMM_BUFFER_SIZE];
> +static char send_buffer[USTCOMM_BUFFER_SIZE];
> +
>   static int epoll_fd;
>   static struct ustcomm_sock *listen_sock;
>
> @@ -72,7 +71,7 @@ static long long make_pidunique(void)
>   {
>   	s64 retval;
>   	struct timeval tv;
> -	
> +
>   	gettimeofday(&tv, NULL);
>
>   	retval = tv.tv_sec;
> @@ -117,6 +116,63 @@ static void print_trace_events(FILE *fp)
>   	unlock_trace_events();
>   }
>
> +static int connect_ustd(void)
> +{
> +	int result, fd;
> +	char default_daemon_path[] = SOCK_DIR "/ustd";
> +	char *explicit_daemon_path, *daemon_path;
> +
> +	explicit_daemon_path = getenv("UST_DAEMON_SOCKET");
> +	if (explicit_daemon_path) {
> +		daemon_path = explicit_daemon_path;
> +	} else {
> +		daemon_path = default_daemon_path;
> +	}
> +
> +	DBG("Connecting to daemon_path %s", daemon_path);
> +
> +	result = ustcomm_connect_path(daemon_path,&fd);
> +	if (result<  0) {
> +		WARN("connect_ustd failed, daemon_path: %s",
> +		     daemon_path);
> +		return result;
> +	}
> +
> +	return fd;
> +}
> +
> +
> +static void request_buffer_consumer(int sock,
> +				   const char *channel,
> +				   int cpu)
> +{
> +	struct ustcomm_header send_header, recv_header;
> +	struct ustcomm_buffer_info buf_inf;
> +	int result = 0;
> +
> +	result = ustcomm_pack_buffer_info(&send_header,
> +					&buf_inf,
> +					  channel,
> +					  cpu);

Just wondering since we've agreed to follow kernel syntax code, is it 
"correct" to have each arguments on a different line?

> +
> +	if (result<  0) {
> +		ERR("failed to pack buffer infor message %s_%d",
> +		    channel, cpu);
> +		return;
> +	}

"buffer infor message" --> typo @ "info"

> +
> +	buf_inf.pid = getpid();
> +	send_header.command = CONSUME_BUFFER;
> +
> +	result = ustcomm_req(sock,&send_header, (char *)&buf_inf,
> +			&recv_header, NULL);
> +	if (result<= 0) {
> +		PERROR("request for buffer consumer failed, is the daemon online?");
> +	}
> +
> +	return;
> +}
> +
>   /* Ask the daemon to collect a trace called trace_name and being
>    * produced by this pid.
>    *
> @@ -126,171 +182,68 @@ static void print_trace_events(FILE *fp)
>
>   static void inform_consumer_daemon(const char *trace_name)
>   {
> -	int i,j;
> +	int sock, i,j;
>   	struct ust_trace *trace;
> -	pid_t pid = getpid();
> -	int result;
> +	const char *ch_name;
> +
> +	sock = connect_ustd();
> +	if (sock<  0) {
> +		return;
> +	}
> +
> +	DBG("Connected to ustd");
>
>   	ltt_lock_traces();
>
>   	trace = _ltt_trace_find(trace_name);
>   	if (trace == NULL) {
>   		WARN("inform_consumer_daemon: could not find trace \"%s\"; it is probably already destroyed", trace_name);
> -		goto finish;
> +		goto unlock_traces;
>   	}
>
>   	for (i=0; i<  trace->nr_channels; i++) {
>   		if (trace->channels[i].request_collection) {
>   			/* iterate on all cpus */
>   			for (j=0; j<trace->channels[i].n_cpus; j++) {
> -				char *buf;
> -				if (asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j)<  0) {
> -                                       ERR("inform_consumer_daemon : asprintf failed (%s_%d)",
> -					   trace->channels[i].channel_name, j);
> -                                       goto finish;
> -				}
> -				result = ustcomm_request_consumer(pid, buf);
> -				if (result == -1) {
> -					WARN("Failed to request collection for channel %s. Is the daemon available?",
> -					     trace->channels[i].channel_name);
> -					/* continue even if fail */
> -				}
> -				free(buf);
> -				STORE_SHARED(buffers_to_export, LOAD_SHARED(buffers_to_export)+1);
> +				ch_name = trace->channels[i].channel_name;
> +				request_buffer_consumer(sock, ch_name, j);
> +				STORE_SHARED(buffers_to_export,
> +					     LOAD_SHARED(buffers_to_export)+1);
>   			}
>   		}
>   	}
>
> -	finish:
> +unlock_traces:
>   	ltt_unlock_traces();
> -}
> -
> -void seperate_channel_cpu(const char *channel_and_cpu, char **channel, int *cpu)
> -{
> -	const char *sep;
> -
> -	sep = rindex(channel_and_cpu, '_');
> -	if (sep == NULL) {
> -		*cpu = -1;
> -		sep = channel_and_cpu + strlen(channel_and_cpu);
> -	} else {
> -		*cpu = atoi(sep+1);
> -	}
>
> -	if (asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu)<  0) {
> -		ERR("seperate_channel_cpu : asprintf failed (%.*s)",
> -		    (int)(sep-channel_and_cpu), channel_and_cpu);
> -		return;
> -	}
> +	close(sock);
>   }
>
> -static int do_cmd_get_shmid(const char *recvbuf, int sock)
> +static struct ust_channel *find_channel(const char *ch_name,
> +					struct ust_trace *trace)
>   {
> -	int retval = 0;
> -	struct ust_trace *trace;
> -	char trace_name[] = "auto";
>   	int i;
> -	char *channel_and_cpu;
> -	int found = 0;
> -	int result;
> -	char *ch_name;
> -	int ch_cpu;
> -
> -	DBG("get_shmid");
> -
> -	channel_and_cpu = nth_token(recvbuf, 1);
> -	if (channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if (ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> -
> -	ltt_lock_traces();
> -	trace = _ltt_trace_find(trace_name);
> -	ltt_unlock_traces();
> -
> -	if (trace == NULL) {
> -		ERR("cannot find trace!");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
>
>   	for (i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -		struct ust_buffer *buf = channel->buf[ch_cpu];
> -
>   		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			char *reply;
> -
> -//			DBG("the shmid for the requested channel is %d", buf->shmid);
> -//			DBG("the shmid for its buffer structure is %d", channel->buf_struct_shmids);
> -			if (asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu])<  0) {
> -				ERR("do_cmd_get_shmid : asprintf failed (%d %d)",
> -				    buf->shmid, channel->buf_struct_shmids[ch_cpu]);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			result = ustcomm_send_reply(reply, sock);
> -			if (result) {
> -				ERR("ustcomm_send_reply failed");
> -				free(reply);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			free(reply);
> -
> -			found = 1;
> -			break;
> +			return&trace->channels[i];
>   		}
>   	}
>
> -	if (!found) {
> -		ERR("channel not found (%s)", channel_and_cpu);
> -	}
> -
> -	free_short_chan_name:
> -	free(ch_name);
> -
> -	end:
> -	return retval;
> +	return NULL;
>   }
>
> -static int do_cmd_get_n_subbufs(const char *recvbuf, int sock)
> +static int get_buffer_shmid_pipe_fd(const char *trace_name, const char *ch_name,
> +				    int ch_cpu,
> +				    int *buf_shmid,
> +				    int *buf_struct_shmid,
> +				    int *buf_pipe_fd)
>   {
> -	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	char *channel_and_cpu;
> -	int found = 0;
> -	int result;
> -	char *ch_name;
> -	int ch_cpu;
> -
> -	DBG("get_n_subbufs");
> -
> -	channel_and_cpu = nth_token(recvbuf, 1);
> -	if (channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> +	struct ust_channel *channel;
> +	struct ust_buffer *buf;
>
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if (ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> +	DBG("get_buffer_shmid_pipe_fd");
>
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find(trace_name);
> @@ -298,122 +251,51 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, int sock)
>
>   	if (trace == NULL) {
>   		ERR("cannot find trace!");
> -		retval = -1;
> -		goto free_short_chan_name;
> +		return -ENODATA;
>   	}
>
> -	for (i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			char *reply;
> -
> -			DBG("the n_subbufs for the requested channel is %d", channel->subbuf_cnt);
> -			if (asprintf(&reply, "%d", channel->subbuf_cnt)<  0) {
> -				ERR("do_cmd_get_n_subbufs : asprintf failed (%d)",
> -				    channel->subbuf_cnt);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			result = ustcomm_send_reply(reply, sock);
> -			if (result) {
> -				ERR("ustcomm_send_reply failed");
> -				free(reply);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			free(reply);
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> -		ERR("unable to find channel");
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
> +		ERR("cannot find channel %s!", ch_name);
> +		return -ENODATA;
>   	}
>
> -	free_short_chan_name:
> -	free(ch_name);
> +	buf = channel->buf[ch_cpu];
>
> -	end:
> -	return retval;
> +	*buf_shmid = buf->shmid;
> +	*buf_struct_shmid = channel->buf_struct_shmids[ch_cpu];
> +	*buf_pipe_fd = buf->data_ready_fd_read;
> +
> +	return 0;
>   }
>
> -static int do_cmd_get_subbuf_size(const char *recvbuf, int sock)
> +static int get_subbuf_num_size(const char *trace_name, const char *ch_name,
> +			       int *num, int *size)
>   {
> -	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	char *channel_and_cpu;
> -	int found = 0;
> -	int result;
> -	char *ch_name;
> -	int ch_cpu;
> +	struct ust_channel *channel;
>
>   	DBG("get_subbuf_size");
>
> -	channel_and_cpu = nth_token(recvbuf, 1);
> -	if (channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if (ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> -
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find(trace_name);
>   	ltt_unlock_traces();
>
> -	if (trace == NULL) {
> +	if (!trace) {
>   		ERR("cannot find trace!");
> -		retval = -1;
> -		goto free_short_chan_name;
> +		return -ENODATA;
>   	}
>
> -	for (i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			char *reply;
> -
> -			DBG("the subbuf_size for the requested channel is %zd", channel->subbuf_size);
> -			if (asprintf(&reply, "%zd", channel->subbuf_size)<  0) {
> -				ERR("do_cmd_get_subbuf_size : asprintf failed (%zd)",
> -				    channel->subbuf_size);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			result = ustcomm_send_reply(reply, sock);
> -			if (result) {
> -				ERR("ustcomm_send_reply failed");
> -				free(reply);
> -				retval = -1;
> -				goto free_short_chan_name;
> -			}
> -
> -			free(reply);
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
>   		ERR("unable to find channel");
> +		return -ENODATA;
>   	}
>
> -	free_short_chan_name:
> -	free(ch_name);
> +	*num = channel->subbuf_cnt;
> +	*size = channel->subbuf_size;
>
> -	end:
> -	return retval;
> +	return 0;
>   }
>
>   /* Return the power of two which is equal or higher to v */
> @@ -429,427 +311,213 @@ static unsigned int pow2_higher_or_eq(unsigned int v)
>   		return retval<<1;
>   }
>
> -static int do_cmd_set_subbuf_size(const char *recvbuf, int sock)
> +static int set_subbuf_size(const char *trace_name, const char *ch_name,
> +			   unsigned int size)
>   {
> -	char *channel_slash_size;
> -	char *ch_name = NULL;
> -	unsigned int size, power;
> +	unsigned int power;
>   	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	int found = 0;
> +	struct ust_channel *channel;
>
>   	DBG("set_subbuf_size");
>
> -	channel_slash_size = nth_token(recvbuf, 1);
> -	sscanf(channel_slash_size, "%a[^/]/%u",&ch_name,&size);
> -
> -	if (ch_name == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
>   	power = pow2_higher_or_eq(size);
>   	power = max_t(unsigned int, 2u, power);
> -	if (power != size)
> +	if (power != size) {
>   		WARN("using the next power of two for buffer size = %u\n", power);
> +	}
>
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find_setup(trace_name);
>   	if (trace == NULL) {
>   		ERR("cannot find trace!");
> -		retval = -1;
> -		goto end;
> +		retval = -ENODATA;
> +		goto unlock_traces;
>   	}
>
> -	for (i = 0; i<  trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -
> -			channel->subbuf_size = power;
> -			DBG("the set_subbuf_size for the requested channel is %zd", channel->subbuf_size);
> -
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
>   		ERR("unable to find channel");
> +		retval = -ENODATA;
> +		goto unlock_traces;
>   	}
>
> -	end:
> +	channel->subbuf_size = power;
> +	DBG("the set_subbuf_size for the requested channel is %zd", channel->subbuf_size);

Just curious here why %zd. I've test it and format ‘%zd’ expects type 
‘signed size_t’ but subbuf_size is unsigned int...

> +
> +unlock_traces:
>   	ltt_unlock_traces();
> -	free(ch_name);
> +
>   	return retval;
>   }
>
> -static int do_cmd_set_subbuf_num(const char *recvbuf, int sock)
> +static int set_subbuf_num(const char *trace_name, const char *ch_name,
> +				 unsigned int num)
>   {
> -	char *channel_slash_num;
> -	char *ch_name = NULL;
> -	unsigned int num;
> -	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	int found = 0;
> +	struct ust_channel *channel;
> +	int retval = 0;
>
>   	DBG("set_subbuf_num");
>
> -	channel_slash_num = nth_token(recvbuf, 1);
> -	sscanf(channel_slash_num, "%a[^/]/%u",&ch_name,&num);
> -
> -	if (ch_name == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
>   	if (num<  2) {
>   		ERR("subbuffer count should be greater than 2");
> -		retval = -1;
> -		goto end;
> +		return -EINVAL;
>   	}
>
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find_setup(trace_name);
>   	if (trace == NULL) {
>   		ERR("cannot find trace!");
> -		retval = -1;
> -		goto end;
> +		retval = -ENODATA;
> +		goto unlock_traces;
>   	}
>
> -	for (i = 0; i<  trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -
> -			channel->subbuf_cnt = num;
> -			DBG("the set_subbuf_cnt for the requested channel is %zd", channel->subbuf_cnt);
> -
> -			found = 1;
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
>   		ERR("unable to find channel");
> +		retval = -ENODATA;
> +		goto unlock_traces;
>   	}
>
> -	end:
> +	channel->subbuf_cnt = num;
> +	DBG("the set_subbuf_cnt for the requested channel is %zd", channel->subbuf_cnt);
> +
> +unlock_traces:
>   	ltt_unlock_traces();
> -	free(ch_name);
>   	return retval;
>   }
>
> -static int do_cmd_get_subbuffer(const char *recvbuf, int sock)
> +static int get_subbuffer(const char *trace_name, const char *ch_name,
> +			 int ch_cpu, long *consumed_old)

I liked the "do_cmd" prefix for "readability" to indicate that this 
function is actually used for a command... don't you think ? (self 
documented code)

Also, libustd.c and this file has now the same function name for a lot 
of them. Don't you think things will get much more confusing ? (or maybe 
you did that on purpose ?)

>   {
> -	int retval = 0, found = 0;;
> -	int i, ch_cpu, result;
> -	long consumed_old = 0;
> +	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	char *channel_and_cpu;
> -	char *ch_name;
> +	struct ust_channel *channel;
> +	struct ust_buffer *buf;
>
>   	DBG("get_subbuf");
>
> -	channel_and_cpu = nth_token(recvbuf, 1);
> -	if(channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if(ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> +	*consumed_old = 0;
>
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find(trace_name);
>
> -	if(trace == NULL) {
> -		int result;
> -
> +	if (!trace) {
>   		DBG("Cannot find trace. It was likely destroyed by the user.");
> -		result = ustcomm_send_reply("NOTFOUND", sock);
> -		if(result) {
> -			ERR("ustcomm_send_reply failed");
> -			retval = -1;
> -			goto unlock_traces;
> -		}
> -
> +		retval = -ENODATA;
>   		goto unlock_traces;
>   	}
>
> -	for(i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if(!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			struct ust_buffer *buf = channel->buf[ch_cpu];
> -			char *reply;
> -
> -			found = 1;
> -
> -			result = ust_buffers_get_subbuf(buf,&consumed_old);
> -			if(result == -EAGAIN) {
> -				WARN("missed buffer?");
> -				retval = 0;
> -
> -				goto unlock_traces;
> -			} else if (result<  0) {
> -				ERR("ust_buffers_get_subbuf: error: %s", strerror(-result));
> -				retval = -1;
> -
> -				goto unlock_traces;
> -			}
> -			if (asprintf(&reply, "%s %ld", "OK", consumed_old)<  0) {
> -				ERR("do_cmd_get_subbuffer: asprintf failed (OK %ld)",
> -				    consumed_old);
> -				retval = -1;
> -
> -				goto unlock_traces;
> -			}
> -			result = ustcomm_send_reply(reply, sock);
> -			if (result<  0) {
> -				ERR("ustcomm_send_reply failed");
> -				free(reply);
> -				retval = -1;
> -
> -				goto unlock_traces;
> -			}
> -			free(reply);
> -
> -			break;
> -		}
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
> +		ERR("unable to find channel");
> +		retval = -ENODATA;
> +		goto unlock_traces;
>   	}
> -	if(found == 0) {
> -		result = ustcomm_send_reply("NOTFOUND", sock);
> -		if (result<= 0) {
> -			ERR("ustcomm_send_reply failed");
> -			retval = -1;
>
> -			goto unlock_traces;
> -		}
> -		ERR("unable to find channel");
> +	buf = channel->buf[ch_cpu];
> +
> +	retval = ust_buffers_get_subbuf(buf, consumed_old);
> +	if(retval<  0) {
> +		WARN("missed buffer?");
>   	}

(just syntax here ) --> if (retval...

>
> -	unlock_traces:
> +unlock_traces:
>   	ltt_unlock_traces();
>
> -	free_short_chan_name:
> -	free(ch_name);
> -
> -	end:
>   	return retval;
>   }
>
>
> -static int do_cmd_get_buffer_fd(const char *recvbuf, int sock)
> +static int notify_buffer_mapped(const char *trace_name,
> +				const char *ch_name,
> +				int ch_cpu)
>   {
>   	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	char *channel_and_cpu;
> -	int found = 0;
> -	char *ch_name;
> -	int ch_cpu;
> -	struct ustcomm_header header;
> +	struct ust_channel *channel;
> +	struct ust_buffer *buf;
>
>   	DBG("get_buffer_fd");
>
> -	channel_and_cpu = nth_token(recvbuf, 1);
> -	if (channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if (ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> -
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find(trace_name);
>
> -	if (trace == NULL) {
> -		int result;
> -
> +	if (!trace) {
> +		retval = -ENODATA;
>   		DBG("Cannot find trace. It was likely destroyed by the user.");
> -		result = ustcomm_send_reply("NOTFOUND", sock);
> -		if (result) {
> -			ERR("ustcomm_send_reply failed");
> -			retval = -1;
> -			goto unlock_traces;
> -		}
> -
>   		goto unlock_traces;
>   	}
>
> -	for (i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			struct ust_buffer *buf = channel->buf[ch_cpu];
> -
> -			found = 1;
> -
> -			header.size = 0;
> -			header.fd_included = 1;
> -			if (ustcomm_send_fd(sock,&header, NULL,
> -					&buf->data_ready_fd_read)<= 0) {
> -				ERR("ustcomm_send_fd failed\n");
> -				goto unlock_traces;
> -			}
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
> +		retval = -ENODATA;
> +		ERR("unable to find channel");
> +		goto unlock_traces;
> +	}
>
> -			/* Being here is the proof the daemon has mapped the buffer in its
> -			 * memory. We may now decrement buffers_to_export.
> -			 */
> -			if (uatomic_read(&buf->consumed) == 0) {
> -				DBG("decrementing buffers_to_export");
> -				STORE_SHARED(buffers_to_export, LOAD_SHARED(buffers_to_export)-1);
> -			}
> +	buf = channel->buf[ch_cpu];
>
> -			/* The buffer has been exported, ergo, we can add it to the
> -			 * list of open buffers
> -			 */
> -			list_add(&buf->open_buffers_list,&open_buffers_list);
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> -		ERR("unable to find channel");
> +	/* Being here is the proof the daemon has mapped the buffer in its
> +	 * memory. We may now decrement buffers_to_export.
> +	 */
> +	if (uatomic_read(&buf->consumed) == 0) {
> +		DBG("decrementing buffers_to_export");
> +		STORE_SHARED(buffers_to_export, LOAD_SHARED(buffers_to_export)-1);
>   	}
>
> -	unlock_traces:
> -	ltt_unlock_traces();
> +	/* The buffer has been exported, ergo, we can add it to the
> +	 * list of open buffers
> +	 */
> +	list_add(&buf->open_buffers_list,&open_buffers_list);
>
> -	free_short_chan_name:
> -	free(ch_name);
> +unlock_traces:
> +	ltt_unlock_traces();
>
> -	end:
>   	return retval;
>   }
>
> -static int do_cmd_put_subbuffer(const char *recvbuf, int sock)
> +static int put_subbuffer(const char *trace_name, const char *ch_name,
> +			 int ch_cpu, long consumed_old)
>   {
>   	int retval = 0;
>   	struct ust_trace *trace;
> -	char trace_name[] = "auto";
> -	int i;
> -	char *channel_and_cpu;
> -	int found = 0;
> -	int result;
> -	char *ch_name;
> -	int ch_cpu;
> -	long consumed_old;
> -	char *consumed_old_str;
> -	char *endptr;
> -	char *reply = NULL;
> +	struct ust_channel *channel;
> +	struct ust_buffer *buf;
>
>   	DBG("put_subbuf");
>
> -	channel_and_cpu = strdup(nth_token(recvbuf, 1));
> -	if (channel_and_cpu == NULL) {
> -		ERR("cannot parse channel");
> -		retval = -1;
> -		goto end;
> -	}
> -
> -	consumed_old_str = strdup(nth_token(recvbuf, 2));
> -	if (consumed_old_str == NULL) {
> -		ERR("cannot parse consumed_old");
> -		retval = -1;
> -		goto free_channel_and_cpu;
> -	}
> -	consumed_old = strtol(consumed_old_str,&endptr, 10);
> -	if (*endptr != '\0') {
> -		ERR("invalid value for consumed_old");
> -		retval = -1;
> -		goto free_consumed_old_str;
> -	}
> -
> -	seperate_channel_cpu(channel_and_cpu,&ch_name,&ch_cpu);
> -	if (ch_cpu == -1) {
> -		ERR("problem parsing channel name");
> -		retval = -1;
> -		goto free_short_chan_name;
> -	}
> -
>   	ltt_lock_traces();
>   	trace = _ltt_trace_find(trace_name);
>
> -	if (trace == NULL) {
> +	if (!trace) {
> +		retval = -ENODATA;
>   		DBG("Cannot find trace. It was likely destroyed by the user.");
> -		result = ustcomm_send_reply("NOTFOUND", sock);
> -		if (result) {
> -			ERR("ustcomm_send_reply failed");
> -			retval = -1;
> -			goto unlock_traces;
> -		}
> -
>   		goto unlock_traces;
>   	}
>
> -	for (i=0; i<trace->nr_channels; i++) {
> -		struct ust_channel *channel =&trace->channels[i];
> -
> -		if (!strcmp(trace->channels[i].channel_name, ch_name)) {
> -			struct ust_buffer *buf = channel->buf[ch_cpu];
> -
> -			found = 1;
> -
> -			result = ust_buffers_put_subbuf(buf, consumed_old);
> -			if (result<  0) {
> -				WARN("ust_buffers_put_subbuf: error (subbuf=%s)", channel_and_cpu);
> -				if (asprintf(&reply, "%s", "ERROR")<  0) {
> -					ERR("do_cmd_put_subbuffer : asprintf failed (ERROR)");
> -					retval = -1;
> -					goto unlock_traces;
> -				}
> -			} else {
> -				DBG("ust_buffers_put_subbuf: success (subbuf=%s)", channel_and_cpu);
> -				if (asprintf(&reply, "%s", "OK")<  0) {
> -					ERR("do_cmd_put_subbuffer : asprintf failed (OK)");
> -					retval = -1;
> -					goto unlock_traces;
> -				}
> -			}
> +	channel = find_channel(ch_name, trace);
> +	if (!channel) {
> +		retval = -ENODATA;
> +		ERR("unable to find channel");
> +		goto unlock_traces;
> +	}
>
> -			result = ustcomm_send_reply(reply, sock);
> -			if (result) {
> -				ERR("ustcomm_send_reply failed");
> -				free(reply);
> -				retval = -1;
> -				goto unlock_traces;
> -			}
> +	buf = channel->buf[ch_cpu];
>
> -			free(reply);
> -			break;
> -		}
> -	}
> -	if (found == 0) {
> -		ERR("unable to find channel");
> +	retval = ust_buffers_put_subbuf(buf, consumed_old);
> +	if (retval<  0) {
> +		WARN("ust_buffers_put_subbuf: error (subbuf=%s_%d)",
> +		     ch_name, ch_cpu);
> +	} else {
> +		DBG("ust_buffers_put_subbuf: success (subbuf=%s_%d)",
> +		    ch_name, ch_cpu);
>   	}
>
> -	unlock_traces:
> +unlock_traces:
>   	ltt_unlock_traces();
> -	free_short_chan_name:
> -	free(ch_name);
> -	free_consumed_old_str:
> -	free(consumed_old_str);
> -	free_channel_and_cpu:
> -	free(channel_and_cpu);
> -
> -	end:
> +
>   	return retval;
>   }
>
> @@ -858,7 +526,7 @@ static void listener_cleanup(void *ptr)
>   	ustcomm_del_named_sock(listen_sock, 0);
>   }
>
> -static void do_cmd_force_switch()
> +static void force_subbuf_switch()
>   {
>   	struct ust_buffer *buf;
>
> @@ -868,69 +536,46 @@ static void do_cmd_force_switch()
>   	}
>   }
>
> -static int process_client_cmd(char *recvbuf, int sock)

This comment applies to the next function (process_simple..)

Have you consider the possibility of arguments given by the client ? As 
an example, if we consider using trace name rather then hardcoded 
"auto", we will need to pass it all the way here...

Ex : lttctl

lttctl -c trace1 -o ...

we still don't have this features in ustctl but maybe think about future 
work ?

> +/* Simple commands are those which need only respond with a return value. */
> +static int process_simple_client_cmd(int command, char *recv_buf)
>   {
>   	int result;
> -	char trace_name[] = "auto";
>   	char trace_type[] = "ustrelay";
> -	int len;
> -
> -	len = strlen(recvbuf);
> -
> -	if (!strcmp(recvbuf, "print_markers")) {
> -		print_markers(stderr);
> -	} else if (!strcmp(recvbuf, "list_markers")) {
> -		char *ptr;
> -		size_t size;
> -		FILE *fp;
> -
> -		fp = open_memstream(&ptr,&size);
> -		print_markers(fp);
> -		fclose(fp);
> -
> -		result = ustcomm_send_reply(ptr, sock);
> -
> -		free(ptr);
> -	} else if (!strcmp(recvbuf, "print_trace_events")) {
> -		print_trace_events(stderr);
> -
> -	} else if (!strcmp(recvbuf, "list_trace_events")) {
> -		char *ptr;
> -		size_t size;
> -		FILE *fp;
> -
> -		fp = open_memstream(&ptr,&size);
> -		if (fp == NULL) {
> -			ERR("opening memstream failed");
> -			return -1;
> -		}
> -		print_trace_events(fp);
> -		fclose(fp);
> +	char trace_name[] = "auto";
>
> -		result = ustcomm_send_reply(ptr, sock);
> -		if (result<  0) {
> -			ERR("list_trace_events failed");
> -			return -1;

Should the following case be indented from the switch(command) ?

> +	switch(command) {
> +	case SET_SOCK_PATH:
> +	{
> +		struct ustcomm_sock_path *sock_msg;
> +		sock_msg = (struct ustcomm_sock_path *)recv_buf;
> +		sock_msg->sock_path =
> +			ustcomm_restore_ptr(sock_msg->sock_path,
> +					    sock_msg->data,
> +					    sizeof(sock_msg->data));
> +		if (!sock_msg->sock_path) {
> +
> +			return -EINVAL;
>   		}
> -		free(ptr);
> -	} else if (!strcmp(recvbuf, "start")) {
> +		return setenv("UST_DAEMON_SOCKET", sock_msg->sock_path, 1);
> +	}
> +	case START:
>   		/* start is an operation that setups the trace, allocates it and starts it */
>   		result = ltt_trace_setup(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_setup failed");
> -			return -1;
> +			return result;
>   		}
>
>   		result = ltt_trace_set_type(trace_name, trace_type);
>   		if (result<  0) {
>   			ERR("ltt_trace_set_type failed");
> -			return -1;
> +			return result;
>   		}
>
>   		result = ltt_trace_alloc(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_alloc failed");
> -			return -1;
> +			return result;
>   		}
>
>   		inform_consumer_daemon(trace_name);
> @@ -938,52 +583,61 @@ static int process_client_cmd(char *recvbuf, int sock)
>   		result = ltt_trace_start(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_start failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (!strcmp(recvbuf, "trace_setup")) {
> +
> +		return 0;
> +	case SETUP_TRACE:
>   		DBG("trace setup");
>
>   		result = ltt_trace_setup(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_setup failed");
> -			return -1;
> +			return result;
>   		}
>
>   		result = ltt_trace_set_type(trace_name, trace_type);
>   		if (result<  0) {
>   			ERR("ltt_trace_set_type failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (!strcmp(recvbuf, "trace_alloc")) {
> +
> +		return 0;
> +	case ALLOC_TRACE:
>   		DBG("trace alloc");
>
>   		result = ltt_trace_alloc(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_alloc failed");
> -			return -1;
> +			return result;
>   		}
>   		inform_consumer_daemon(trace_name);
> -	} else if (!strcmp(recvbuf, "trace_create")) {
> +
> +		return 0;
> +
> +	case CREATE_TRACE:
>   		DBG("trace create");
>
>   		result = ltt_trace_setup(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_setup failed");
> -			return -1;
> +			return result;
>   		}
>
>   		result = ltt_trace_set_type(trace_name, trace_type);
>   		if (result<  0) {
>   			ERR("ltt_trace_set_type failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (!strcmp(recvbuf, "trace_start")) {
> +
> +		return 0;
> +	case START_TRACE:
>   		DBG("trace start");
>
>   		result = ltt_trace_alloc(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_alloc failed");
> -			return -1;
> +			return result;
>   		}
>   		if (!result) {

I'm wondering here if it's correct... if result = -1, we still execute 
inform_consumer_daemon... on a quick look, I don't think we want that ??

>   			inform_consumer_daemon(trace_name);
> @@ -992,138 +646,381 @@ static int process_client_cmd(char *recvbuf, int sock)
>   		result = ltt_trace_start(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_start failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (!strcmp(recvbuf, "trace_stop")) {
> +
> +		return 0;
> +	case STOP_TRACE:
>   		DBG("trace stop");
>
>   		result = ltt_trace_stop(trace_name);
>   		if (result<  0) {
>   			ERR("ltt_trace_stop failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (!strcmp(recvbuf, "trace_destroy")) {
>
> +		return 0;
> +	case DESTROY_TRACE:
>   		DBG("trace destroy");
>
>   		result = ltt_trace_destroy(trace_name, 0);
>   		if (result<  0) {
>   			ERR("ltt_trace_destroy failed");
> -			return -1;
> +			return result;
>   		}
> -	} else if (nth_token_is(recvbuf, "get_shmid", 0) == 1) {
> -		do_cmd_get_shmid(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "get_n_subbufs", 0) == 1) {
> -		do_cmd_get_n_subbufs(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "get_subbuf_size", 0) == 1) {
> -		do_cmd_get_subbuf_size(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "load_probe_lib", 0) == 1) {
> -		char *libfile;
> +		return 0;
> +	case FORCE_SUBBUF_SWITCH:
> +		force_subbuf_switch();

I think a future patch should be done for a return code about this 
function... a fail switch will go unotice :(

>
> -		libfile = nth_token(recvbuf, 1);
> +		break;
>
> -		DBG("load_probe_lib loading %s", libfile);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void process_channel_cmd(int sock, int command,
> +				struct ustcomm_channel_info *ch_inf)
> +{
> +	struct ustcomm_header _reply_header;
> +	struct ustcomm_header *reply_header =&_reply_header;
> +	struct ustcomm_channel_info *reply_msg =
> +		(struct ustcomm_channel_info *)send_buffer;
> +	char trace_name[] = "auto";
> +	int result, offset = 0, num, size;
> +
> +	memset(reply_header, 0, sizeof(*reply_header));
> +
> +	switch (command) {
> +	case GET_SUBBUF_NUM_SIZE:
> +		result = get_subbuf_num_size(trace_name,
> +					     ch_inf->channel,
> +					&num,&size);
> +		if (result<  0) {
> +			reply_header->result = result;
> +			break;
> +		}
> +
> +		reply_msg->channel = USTCOMM_POISON_PTR;
> +		reply_msg->subbuf_num = num;
> +		reply_msg->subbuf_size = size;
> +
> +
> +		reply_header->size = COMPUTE_MSG_SIZE(reply_msg, offset);
> +
> +		break;
> +	case SET_SUBBUF_NUM:
> +		reply_header->result = set_subbuf_num(trace_name,
> +						      ch_inf->channel,
> +						      ch_inf->subbuf_num);
> +
> +		break;
> +	case SET_SUBBUF_SIZE:
> +		reply_header->result = set_subbuf_size(trace_name,
> +						       ch_inf->channel,
> +						       ch_inf->subbuf_size);
> +
> +
> +		break;
> +	}
> +	if (ustcomm_send(sock, reply_header, (char *)reply_msg)<  0) {
> +		ERR("ustcomm_send failed");
> +	}
> +}
> +
> +static void process_buffer_cmd(int sock, int command,
> +			       struct ustcomm_buffer_info *buf_inf)
> +{
> +	struct ustcomm_header _reply_header;
> +	struct ustcomm_header *reply_header =&_reply_header;
> +	struct ustcomm_buffer_info *reply_msg =
> +		(struct ustcomm_buffer_info *)send_buffer;
> +	char trace_name[] = "auto";
> +	int result, offset = 0, buf_shmid, buf_struct_shmid, buf_pipe_fd;
> +	long consumed_old;
> +
> +	memset(reply_header, 0, sizeof(*reply_header));
>
> -		free(libfile);
> -	} else if (nth_token_is(recvbuf, "get_subbuffer", 0) == 1) {
> -		do_cmd_get_subbuffer(recvbuf, sock);
> -	}
> -	else if(nth_token_is(recvbuf, "get_buffer_fd", 0) == 1) {
> -		do_cmd_get_buffer_fd(recvbuf, sock);
> -	}
> -	else if(nth_token_is(recvbuf, "put_subbuffer", 0) == 1) {
> -		do_cmd_put_subbuffer(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "set_subbuf_size", 0) == 1) {
> -		do_cmd_set_subbuf_size(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "set_subbuf_num", 0) == 1) {
> -		do_cmd_set_subbuf_num(recvbuf, sock);
> -	} else if (nth_token_is(recvbuf, "enable_marker", 0) == 1) {
> -		char *channel_slash_name = nth_token(recvbuf, 1);
> -		char *channel_name = NULL;
> -		char *marker_name = NULL;
> -
> -		result = sscanf(channel_slash_name, "%a[^/]/%as",&channel_name,&marker_name);
> -
> -		if (channel_name == NULL || marker_name == NULL) {
> -			WARN("invalid marker name");
> -			free(channel_name);
> -			free(marker_name);
> -			goto next_cmd;
> +	switch (command) {
> +	case GET_BUF_SHMID_PIPE_FD:
> +		result = get_buffer_shmid_pipe_fd(trace_name, buf_inf->channel,
> +						  buf_inf->ch_cpu,
> +						&buf_shmid,
> +						&buf_struct_shmid,
> +						&buf_pipe_fd);
> +		if (result<  0) {
> +			reply_header->result = result;
> +			break;
>   		}
>
> -		result = ltt_marker_connect(channel_name, marker_name, "default");
> +		reply_msg->channel = USTCOMM_POISON_PTR;
> +		reply_msg->buf_shmid = buf_shmid;
> +		reply_msg->buf_struct_shmid = buf_struct_shmid;
> +
> +		reply_header->size = COMPUTE_MSG_SIZE(reply_msg, offset);
> +		reply_header->fd_included = 1;
> +
> +		if (ustcomm_send_fd(sock, reply_header, (char *)reply_msg,
> +				&buf_pipe_fd)<  0) {
> +			ERR("ustcomm_send failed");
> +		}
> +		return;
> +
> +	case NOTIFY_BUF_MAPPED:
> +		reply_header->result =
> +			notify_buffer_mapped(trace_name,
> +					     buf_inf->channel,
> +					     buf_inf->ch_cpu);
> +		break;
> +	case GET_SUBBUFFER:
> +		result = get_subbuffer(trace_name, buf_inf->channel,
> +				       buf_inf->ch_cpu,&consumed_old);
>   		if (result<  0) {
> -			WARN("could not enable marker; channel=%s, name=%s", channel_name, marker_name);
> +			reply_header->result = result;
> +			break;
>   		}
>
> -		free(channel_name);
> -		free(marker_name);
> -	} else if (nth_token_is(recvbuf, "disable_marker", 0) == 1) {
> -		char *channel_slash_name = nth_token(recvbuf, 1);
> -		char *marker_name = NULL;
> -		char *channel_name = NULL;
> +		reply_msg->channel = USTCOMM_POISON_PTR;
> +		reply_msg->consumed_old = consumed_old;
> +
> +		reply_header->size = COMPUTE_MSG_SIZE(reply_msg, offset);
> +
> +		break;
> +	case PUT_SUBBUFFER:
> +		result = put_subbuffer(trace_name, buf_inf->channel,
> +				       buf_inf->ch_cpu,
> +				       buf_inf->consumed_old);
> +		reply_header->result = result;
> +
> +		break;
> +	}
> +
> +	if (ustcomm_send(sock, reply_header, (char *)reply_msg)<  0) {
> +		ERR("ustcomm_send failed");
> +	}
>
> -		result = sscanf(channel_slash_name, "%a[^/]/%as",&channel_name,&marker_name);
> +}
> +
> +static void process_marker_cmd(int sock, int command,
> +			       struct ustcomm_marker_info *marker_inf)
> +{
> +	struct ustcomm_header _reply_header;
> +	struct ustcomm_header *reply_header =&_reply_header;
> +	int result;
>
> -		if (channel_name == NULL || marker_name == NULL) {
> -			WARN("invalid marker name");
> -			free(channel_name);
> -			free(marker_name);
> -			goto next_cmd;
> +	memset(reply_header, 0, sizeof(*reply_header));
> +
> +	switch(command) {
> +	case ENABLE_MARKER:
> +
> +		result = ltt_marker_connect(marker_inf->channel,
> +					    marker_inf->marker,
> +					    "default");
> +		if (result<  0) {
> +			WARN("could not enable marker; channel=%s,"
> +			     " name=%s",
> +			     marker_inf->channel,
> +			     marker_inf->marker);
> +
> +		}
> +		break;
> +	case DISABLE_MARKER:
> +		result = ltt_marker_disconnect(marker_inf->channel,
> +					       marker_inf->marker,
> +					       "default");
> +		if (result<  0) {
> +			WARN("could not disable marker; channel=%s,"
> +			     " name=%s",
> +			     marker_inf->channel,
> +			     marker_inf->marker);
>   		}
> +		break;
> +	}
> +
> +	reply_header->result = result;
> +
> +	if (ustcomm_send(sock, reply_header, NULL)<  0) {
> +		ERR("ustcomm_send failed");
> +	}
>
> -		result = ltt_marker_disconnect(channel_name, marker_name, "default");
> +}
> +static void process_client_cmd(struct ustcomm_header *recv_header,
> +			       char *recv_buf, int sock)
> +{
> +	int result;
> +	struct ustcomm_header _reply_header;
> +	struct ustcomm_header *reply_header =&_reply_header;
> +	char *send_buf = send_buffer;
> +
> +	memset(reply_header, 0, sizeof(*reply_header));
> +	memset(send_buf, 0, sizeof(send_buffer));
> +
> +	switch(recv_header->command) {
> +	case GET_SUBBUF_NUM_SIZE:
> +	case SET_SUBBUF_NUM:
> +	case SET_SUBBUF_SIZE:
> +	{
> +		struct ustcomm_channel_info *ch_inf;
> +		ch_inf = (struct ustcomm_channel_info *)recv_buf;
> +		result = ustcomm_unpack_channel_info(ch_inf);
> +		if (result<  0) {
> +			ERR("couldn't unpack channel info");
> +			reply_header->result = -EINVAL;
> +			goto send_response;
> +		}
> +		process_channel_cmd(sock, recv_header->command, ch_inf);
> +		return;
> +	}
> +	case GET_BUF_SHMID_PIPE_FD:
> +	case NOTIFY_BUF_MAPPED:
> +	case GET_SUBBUFFER:
> +	case PUT_SUBBUFFER:
> +	{
> +		struct ustcomm_buffer_info *buf_inf;
> +		buf_inf = (struct ustcomm_buffer_info *)recv_buf;
> +		result = ustcomm_unpack_buffer_info(buf_inf);
> +		if (result<  0) {
> +			ERR("couldn't unpack buffer info");
> +			reply_header->result = -EINVAL;
> +			goto send_response;
> +		}
> +		process_buffer_cmd(sock, recv_header->command, buf_inf);
> +		return;
> +	}
> +	case ENABLE_MARKER:
> +	case DISABLE_MARKER:
> +	{
> +		struct ustcomm_marker_info *marker_inf;
> +		marker_inf = (struct ustcomm_marker_info *)recv_buf;
> +		result = ustcomm_unpack_marker_info(marker_inf);
>   		if (result<  0) {
> -			WARN("could not disable marker; channel=%s, name=%s", channel_name, marker_name);
> +			ERR("couldn't unpack marker info");
> +			reply_header->result = -EINVAL;
> +			goto send_response;
>   		}
> +		process_marker_cmd(sock, recv_header->command, marker_inf);
> +		return;
> +	}
> +	case LIST_MARKERS:
> +	{
> +		char *ptr;
> +		size_t size;
> +		FILE *fp;
>
> -		free(channel_name);
> -		free(marker_name);
> -	} else if (nth_token_is(recvbuf, "get_pidunique", 0) == 1) {
> -		char *reply;
> +		fp = open_memstream(&ptr,&size);
> +		if (fp == NULL) {
> +			ERR("opening memstream failed");
> +			return;
> +		}
> +		print_markers(fp);
> +		fclose(fp);
> +
> +		reply_header->size = size;
>
> -		if (asprintf(&reply, "%lld", pidunique)<  0) {
> -			ERR("process_client_cmd : asprintf failed (%lld)",
> -			    pidunique);
> -			goto next_cmd;
> +		result = ustcomm_send(sock, reply_header, ptr);
> +
> +		free(ptr);
> +
> +		if (result<  0) {
> +			PERROR("failed to send markers list");
>   		}
>
> -		result = ustcomm_send_reply(reply, sock);
> -		if (result) {
> -			ERR("listener: get_pidunique: ustcomm_send_reply failed");
> -			goto next_cmd;
> +		break;
> +	}
> +	case LIST_TRACE_EVENTS:
> +	{
> +		char *ptr;
> +		size_t size;
> +		FILE *fp;
> +
> +		fp = open_memstream(&ptr,&size);
> +		if (fp == NULL) {
> +			ERR("opening memstream failed");
> +			return;
>   		}
> +		print_trace_events(fp);
> +		fclose(fp);
>
> -		free(reply);
> -	} else if (nth_token_is(recvbuf, "get_sock_path", 0) == 1) {
> -		char *reply = getenv("UST_DAEMON_SOCKET");
> -		if (!reply) {
> -			if (asprintf(&reply, "%s/%s", SOCK_DIR, "ustd")<  0) {
> -				ERR("process_client_cmd : asprintf failed (%s/ustd)",
> -				    SOCK_DIR);
> -				goto next_cmd;
> -			}
> -			result = ustcomm_send_reply(reply, sock);
> -			free(reply);
> +		reply_header->size = size;
> +
> +		result = ustcomm_send(sock, reply_header, ptr);
> +
> +		free(ptr);
> +
> +		if (result<  0) {
> +			ERR("list_trace_events failed");
> +			return;
> +		}
> +
> +		break;
> +	}
> +	case LOAD_PROBE_LIB:
> +	{
> +		char *libfile;
> +
> +		/* FIXME: No functionality at all... */
> +		libfile = recv_buf;
> +
> +		DBG("load_probe_lib loading %s", libfile);
> +
> +		break;
> +	}
> +	case GET_PIDUNIQUE:
> +	{
> +		struct ustcomm_pidunique *pid_msg;
> +		pid_msg = (struct ustcomm_pidunique *)send_buf;
> +
> +		pid_msg->pidunique = pidunique;
> +		reply_header->size = sizeof(pid_msg);
> +
> +		goto send_response;
> +
> +	}
> +	case GET_SOCK_PATH:
> +	{
> +		struct ustcomm_sock_path *sock_msg;
> +		int offset;
> +		char *sock_path_env;
> +
> +		sock_msg = (struct ustcomm_sock_path *)send_buf;
> +
> +		sock_path_env = getenv("UST_DAEMON_SOCKET");
> +
> +		if (!sock_path_env) {
> +			sock_msg->sock_path
> +				= ustcomm_print_data(sock_msg->data,
> +						     sizeof(sock_msg->data),
> +						&offset,
> +						     "%s/%s", SOCK_DIR, "ustd");
>   		} else {
> -			result = ustcomm_send_reply(reply, sock);
> +			sock_msg->sock_path
> +				= ustcomm_print_data(sock_msg->data,
> +						     sizeof(sock_msg->data),
> +						&offset,
> +						     sock_path_env);
>   		}
> -		if (result)
> -			ERR("ustcomm_send_reply failed");
> -	} else if (nth_token_is(recvbuf, "set_sock_path", 0) == 1) {
> -		char *sock_path = nth_token(recvbuf, 1);
> -		result = setenv("UST_DAEMON_SOCKET", sock_path, 1);
> -		if (result)
> -			ERR("cannot set UST_DAEMON_SOCKET environment variable");
> -	} else if (nth_token_is(recvbuf, "force_switch", 0) == 1) {
> -		do_cmd_force_switch();
> -	} else {
> -		ERR("unable to parse message: %s", recvbuf);
> +		if (sock_msg->sock_path == USTCOMM_POISON_PTR) {
> +			reply_header->result = -ENOMEM;
> +		} else {
> +			reply_header->size = COMPUTE_MSG_SIZE(sock_msg, offset);
> +		}
> +
> +		goto send_response;
>   	}
> +	default:
> +		reply_header->result =
> +			process_simple_client_cmd(recv_header->command,
> +						  recv_buf);
> +		goto send_response;
>
> -next_cmd:
> +	}
>
> -	return 0;
> +	return;
> +
> +send_response:
> +	ustcomm_send(sock, reply_header, send_buf);
>   }
>
>   #define MAX_EVENTS 10
> @@ -1160,13 +1057,19 @@ void *listener_main(void *p)
>   				ustcomm_init_sock(accept_fd, epoll_fd,
>   						&ust_socks);
>   			} else {
> -				char *msg = NULL;
> -				result = recv_message_conn(epoll_sock->fd,&msg);
> +				memset(receive_header, 0,
> +				       sizeof(*receive_header));
> +				memset(receive_buffer, 0,
> +				       sizeof(receive_buffer));
> +				result = ustcomm_recv(epoll_sock->fd,
> +						      receive_header,
> +						      receive_buffer);
>   				if (result == 0) {
>   					ustcomm_del_sock(epoll_sock, 0);
> -				} else if (msg) {
> -					process_client_cmd(msg, epoll_sock->fd);
> -					free(msg);
> +				} else {
> +					process_client_cmd(receive_header,
> +							   receive_buffer,
> +							   epoll_sock->fd);
>   				}
>   			}
>   		}
> @@ -1528,13 +1431,13 @@ static int trace_recording(void)
>
>   int restarting_usleep(useconds_t usecs)
>   {
> -        struct timespec tv;
> -        int result;
> -
> -        tv.tv_sec = 0;
> -        tv.tv_nsec = usecs * 1000;
> -
> -        do {
> +        struct timespec tv;
> +        int result;
> +
> +        tv.tv_sec = 0;
> +        tv.tv_nsec = usecs * 1000;
> +
> +        do {
>                   result = nanosleep(&tv,&tv);
>           } while (result == -1&&  errno == EINTR);
>
> diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
> index ac90f6c..3a83eac 100644
> --- a/libustcmd/ustcmd.c
> +++ b/libustcmd/ustcmd.c
> @@ -28,6 +28,61 @@
>   #include "ust/ustcmd.h"
>   #include "usterr.h"
>
> +static int do_cmd(const pid_t pid,
> +		  const struct ustcomm_header *req_header,
> +		  const char *req_data,
> +		  struct ustcomm_header *res_header,
> +		  char **res_data)
> +{
> +	int app_fd, result, saved_errno = 0;
> +	char *recv_buf;
> +
> +	if (ustcomm_connect_app(pid,&app_fd)) {
> +		ERR("could not connect to PID %u", (unsigned int) pid);
> +		errno = ENOTCONN;
> +		return -1;
> +	}
> +
> +	recv_buf = malloc(USTCOMM_BUFFER_SIZE);

zmalloc

> +	if (!recv_buf) {
> +		saved_errno = ENOMEM;
> +		goto close_app_fd;
> +	}
> +
> +	result = ustcomm_req(app_fd, req_header, req_data, res_header, recv_buf);
> +	if (result>  0) {
> +		saved_errno = -res_header->result;
> +		if (res_header->size == 0 || saved_errno>  0) {
> +			free(recv_buf);
> +		} else {
> +			if (res_data) {
> +				*res_data = recv_buf;
> +			} else {
> +				free(recv_buf);
> +			}
> +		}
> +	} else {
> +		ERR("ustcomm req failed");
> +		if (result == 0) {
> +			saved_errno = ENOTCONN;
> +		} else {
> +			saved_errno = -result;
> +		}
> +		free(recv_buf);
> +	}
> +
> +close_app_fd:
> +	close(app_fd);
> +
> +	errno = saved_errno;
> +
> +	if (errno) {
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   pid_t *ustcmd_get_online_pids(void)
>   {
>   	struct dirent *dirent;
> @@ -43,13 +98,13 @@ pid_t *ustcmd_get_online_pids(void)
>
>   	while ((dirent = readdir(dir))) {
>   		if (!strcmp(dirent->d_name, ".") ||
> -			!strcmp(dirent->d_name, "..")) {
> +		    !strcmp(dirent->d_name, "..")) {
>
>   			continue;
>   		}
>
>   		if (dirent->d_type != DT_DIR&&
> -			!!strcmp(dirent->d_name, "ustd")) {
> +		    !!strcmp(dirent->d_name, "ustd")) {
>
>   			sscanf(dirent->d_name, "%u", (unsigned int *)&ret[i]);
>   			/* FIXME: Here we previously called pid_is_online, which
> @@ -68,7 +123,7 @@ pid_t *ustcmd_get_online_pids(void)
>   	ret[i] = 0; /* Array end */
>
>   	if (ret[0] == 0) {
> -		 /* No PID at all */
> +		/* No PID at all */
>   		free(ret);
>   		return NULL;
>   	}
> @@ -85,30 +140,26 @@ pid_t *ustcmd_get_online_pids(void)
>    * @param pid	Traced process ID
>    * @return	0 if successful, or errors {USTCMD_ERR_GEN, USTCMD_ERR_ARG}
>    */
> -int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
> +int ustcmd_set_marker_state(const char *channel, const char *marker,
> +			    int state, pid_t pid)
>   {
> -	char *cmd_str [] = {"disable_marker", "enable_marker"};
> -	char *cmd;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_marker_info marker_inf;
>   	int result;
>
> -	if (mn == NULL) {
> -		return USTCMD_ERR_ARG;
> -	}
> -
> -	if (asprintf(&cmd, "%s %s", cmd_str[state], mn)<  0) {
> -		ERR("ustcmd_set_marker_state : asprintf failed (%s %s)",
> -		    cmd_str[state], mn);
> -		return USTCMD_ERR_GEN;
> +	result = ustcomm_pack_marker_info(&req_header,
> +					&marker_inf,
> +					  channel,
> +					  marker);
> +	if (result<  0) {
> +		errno = -result;
> +		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid, NULL);
> -	if (result != 1) {
> -		free(cmd);
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = state ? ENABLE_MARKER : DISABLE_MARKER;
>
> -	free(cmd);
> -	return 0;
> +	return do_cmd(pid,&req_header, (char *)&marker_inf,
> +		&res_header, NULL);
>   }
>
>   /**
> @@ -118,25 +169,26 @@ int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
>    * @param pid		Traced process ID
>    * @return		0 if successful, or error
>    */
> -int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
> +int ustcmd_set_subbuf_size(const char *channel, unsigned int subbuf_size,
> +			   pid_t pid)
>   {
> -	char *cmd;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_channel_info ch_inf;
>   	int result;
>
> -	if (asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size)<  0) {
> -		ERR("ustcmd_set_subbuf_size : asprintf failed (set_subbuf_size %s)",
> -		    channel_size);
> +	result = ustcomm_pack_channel_info(&req_header,
> +					&ch_inf,
> +					   channel);
> +	if (result<  0) {
> +		errno = -result;
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid, NULL);
> -	if (result != 1) {
> -		free(cmd);
> -		return 1;
> -	}
> +	req_header.command = SET_SUBBUF_SIZE;
> +	ch_inf.subbuf_size = subbuf_size;
>
> -	free(cmd);
> -	return 0;
> +	return do_cmd(pid,&req_header, (char *)&ch_inf,
> +		&res_header, NULL);
>   }
>
>   /**
> @@ -146,56 +198,59 @@ int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
>    * @param pid		Traced process ID
>    * @return		0 if successful, or error
>    */
> -int ustcmd_set_subbuf_num(const char *channel_size, pid_t pid)
> +int ustcmd_set_subbuf_num(const char *channel, unsigned int num,
> +			  pid_t pid)
>   {
> -	char *cmd;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_channel_info ch_inf;
>   	int result;
>
> -	if (asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size)<  0) {
> -		ERR("ustcmd_set_subbuf_num : asprintf failed (set_subbuf_num %s",
> -		    channel_size);
> +	result = ustcomm_pack_channel_info(&req_header,
> +					&ch_inf,
> +					   channel);
> +	if (result<  0) {
> +		errno = -result;
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid, NULL);
> -	if (result != 1) {
> -		free(cmd);
> -		return 1;
> -	}
> +	req_header.command = SET_SUBBUF_NUM;
> +	ch_inf.subbuf_num = num;
> +
> +	return do_cmd(pid,&req_header, (char *)&ch_inf,
> +		&res_header, NULL);
>
> -	free(cmd);
> -	return 0;
>   }
>
> -/**
> - * Get subbuffer size.
> - *
> - * @param channel	Channel name
> - * @param pid		Traced process ID
> - * @return		subbuf size if successful, or error
> - */
> -int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
> +static int ustcmd_get_subbuf_num_size(const char *channel, pid_t pid,
> +			       int *num, int *size)
>   {
> -	char *cmd, *reply;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_channel_info ch_inf, *ch_inf_res;
>   	int result;
>
> -	/* format: channel_cpu */
> -	if (asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel)<  0) {
> -		ERR("ustcmd_get_subbuf_size : asprintf failed (get_subbuf_size, %s_0",
> -		    channel);
> +
> +	result = ustcomm_pack_channel_info(&req_header,
> +					&ch_inf,
> +					   channel);
> +	if (result<  0) {
> +		errno = -result;
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid,&reply);
> -	if (result != 1) {
> -		free(cmd);
> +	req_header.command = GET_SUBBUF_NUM_SIZE;
> +
> +	result = do_cmd(pid,&req_header, (char *)&ch_inf,
> +			&res_header, (char **)&ch_inf_res);
> +	if (result<  0) {
>   		return -1;
>   	}
>
> -	result = atoi(reply);
> -	free(cmd);
> -	free(reply);
> -	return result;
> +	*num = ch_inf_res->subbuf_num;
> +	*size = ch_inf_res->subbuf_size;
> +
> +	free(ch_inf_res);
> +
> +	return 0;
>   }
>
>   /**
> @@ -207,26 +262,37 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
>    */
>   int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
>   {
> -	char *cmd, *reply;
> -	int result;
> +	int num, size, result;
>
> -	/* format: channel_cpu */
> -	if (asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel)<  0) {
> -		ERR("ustcmd_get_subbuf_num : asprintf failed (get_n_subbufs, %s_0",
> -		    channel);
> +	result = ustcmd_get_subbuf_num_size(channel, pid,
> +					&num,&size);
> +	if (result<  0) {
> +		errno = -result;
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid,&reply);
> -	if (result != 1) {
> -		free(cmd);
> +	return num;
> +}
> +
> +/**
> + * Get subbuffer size.
> + *
> + * @param channel	Channel name
> + * @param pid		Traced process ID
> + * @return		subbuf size if successful, or error
> + */
> +int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
> +{
> +	int num, size, result;
> +
> +	result = ustcmd_get_subbuf_num_size(channel, pid,
> +					&num,&size);
> +	if (result<  0) {
> +		errno = -result;
>   		return -1;
>   	}
>
> -	result = atoi(reply);
> -	free(cmd);
> -	free(reply);
> -	return result;
> +	return size;
>   }
>
>   /**
> @@ -237,14 +303,12 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
>    */
>   int ustcmd_destroy_trace(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("trace_destroy", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = DESTROY_TRACE;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -255,14 +319,12 @@ int ustcmd_destroy_trace(pid_t pid)
>    */
>   int ustcmd_setup_and_start(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("start", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = START;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -273,14 +335,12 @@ int ustcmd_setup_and_start(pid_t pid)
>    */
>   int ustcmd_create_trace(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("trace_create", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = CREATE_TRACE;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -291,14 +351,12 @@ int ustcmd_create_trace(pid_t pid)
>    */
>   int ustcmd_start_trace(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("trace_start", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = START_TRACE;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -309,14 +367,12 @@ int ustcmd_start_trace(pid_t pid)
>    */
>   int ustcmd_alloc_trace(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("trace_alloc", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = ALLOC_TRACE;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -327,14 +383,12 @@ int ustcmd_alloc_trace(pid_t pid)
>    */
>   int ustcmd_stop_trace(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("trace_stop", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = STOP_TRACE;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
>   /**
> @@ -391,22 +445,40 @@ int ustcmd_free_cmsf(struct marker_status *cmsf)
>    */
>   int ustcmd_get_cmsf(struct marker_status **cmsf, const pid_t pid)
>   {
> +	struct ustcomm_header req_header, res_header;
>   	char *big_str = NULL;
> -	int result;
> +	int result, app_fd;
>   	struct marker_status *tmp_cmsf = NULL;
>   	unsigned int i = 0, cmsf_ind = 0;
>
>   	if (cmsf == NULL) {
>   		return -1;
>   	}
> -	result = ustcmd_send_cmd("list_markers", pid,&big_str);
> -	if (result != 1) {
> -		ERR("error while getting markers list");
> +
> +	if (ustcomm_connect_app(pid,&app_fd)) {
> +		ERR("could not connect to PID %u", (unsigned int) pid);
> +		return -1;
> +	}
> +
> +	req_header.command = LIST_MARKERS;
> +	req_header.size = 0;
> +
> +	result = ustcomm_send(app_fd,&req_header, NULL);
> +	if (result<= 0) {
> +		PERROR("error while requesting markers list for process %d", pid);
> +		return -1;
> +	}
> +
> +	result = ustcomm_recv_alloc(app_fd,&res_header,&big_str);
> +	if (result<= 0) {
> +		ERR("error while receiving markers list");
>   		return -1;
>   	}
>
> +	close(app_fd);
> +
>   	tmp_cmsf = (struct marker_status *) malloc(sizeof(struct marker_status) *
> -		(ustcmd_count_nl(big_str) + 1));

zmalloc

> +						   (ustcmd_count_nl(big_str) + 1));
>   	if (tmp_cmsf == NULL) {
>   		ERR("Failed to allocate CMSF array");
>   		return -1;
> @@ -417,12 +489,12 @@ int ustcmd_get_cmsf(struct marker_status **cmsf, const pid_t pid)
>   		char state;
>
>   		sscanf(big_str + i, "marker: %a[^/]/%a[^ ] %c %a[^\n]",
> -			&tmp_cmsf[cmsf_ind].channel,
> -			&tmp_cmsf[cmsf_ind].marker,
> -			&state,
> -			&tmp_cmsf[cmsf_ind].fs);
> +		&tmp_cmsf[cmsf_ind].channel,
> +		&tmp_cmsf[cmsf_ind].marker,
> +		&state,
> +		&tmp_cmsf[cmsf_ind].fs);
>   		tmp_cmsf[cmsf_ind].state = (state == USTCMD_MS_CHR_ON ?
> -			USTCMD_MS_ON : USTCMD_MS_OFF); /* Marker state */
> +					    USTCMD_MS_ON : USTCMD_MS_OFF); /* Marker state */
>
>   		while (big_str[i] != '\n') {
>   			++i; /* Go to next '\n' */
> @@ -472,10 +544,11 @@ int ustcmd_free_tes(struct trace_event_status *tes)
>    * @return	0 if successful, or -1 on error
>    */
>   int ustcmd_get_tes(struct trace_event_status **tes,
> -			    const pid_t pid)
> +		   const pid_t pid)
>   {
> +	struct ustcomm_header req_header, res_header;
>   	char *big_str = NULL;
> -	int result;
> +	int result, app_fd;
>   	struct trace_event_status *tmp_tes = NULL;
>   	unsigned int i = 0, tes_ind = 0;
>
> @@ -483,12 +556,28 @@ int ustcmd_get_tes(struct trace_event_status **tes,
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd("list_trace_events", pid,&big_str);
> +	if (ustcomm_connect_app(pid,&app_fd)) {
> +		ERR("could not connect to PID %u", (unsigned int) pid);
> +		return -1;
> +	}
> +
> +	req_header.command = LIST_TRACE_EVENTS;
> +	req_header.size = 0;
> +
> +	result = ustcomm_send(app_fd,&req_header, NULL);
>   	if (result != 1) {
> -		ERR("error while getting trace_event list");
> +		ERR("error while requesting trace_event list");
>   		return -1;
>   	}
>
> +	result = ustcomm_recv_alloc(app_fd,&res_header,&big_str);
> +	if (result != 1) {
> +		ERR("error while receiving markers list");
> +		return -1;
> +	}
> +
> +	close(app_fd);
> +
>   	tmp_tes = (struct trace_event_status *)
>   		zmalloc(sizeof(struct trace_event_status) *
>   			(ustcmd_count_nl(big_str) + 1));
> @@ -499,10 +588,8 @@ int ustcmd_get_tes(struct trace_event_status **tes,
>
>   	/* Parse received reply string (format: "[name]"): */
>   	while (big_str[i] != '\0') {
> -		char state;
> -
>   		sscanf(big_str + i, "trace_event: %a[^\n]",
> -			&tmp_tes[tes_ind].name);
> +		&tmp_tes[tes_ind].name);
>   		while (big_str[i] != '\n') {
>   			++i; /* Go to next '\n' */
>   		}
> @@ -526,23 +613,23 @@ int ustcmd_get_tes(struct trace_event_status **tes,
>    */
>   int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
>   {
> -	char *cmd;
> -	int result;
> -
> -	if (asprintf(&cmd, "%s %s", "set_sock_path", sock_path)<  0) {
> -		ERR("ustcmd_set_sock_path : asprintf failed (set_sock_path, %s",
> -		    sock_path);
> +	int offset = 0;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_sock_path sock_path_msg;
> +
> +	sock_path_msg.sock_path = ustcomm_print_data(sock_path_msg.data,
> +						     sizeof(sock_path_msg.data),
> +						&offset,
> +						     sock_path);
> +	if (sock_path_msg.sock_path == USTCOMM_POISON_PTR) {
>   		return -1;
>   	}
>
> -	result = ustcmd_send_cmd(cmd, pid, NULL);
> -	if (result != 1) {
> -		free(cmd);
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = SET_SOCK_PATH;
> +	req_header.size = COMPUTE_MSG_SIZE(&sock_path_msg, offset);
>
> -	free(cmd);
> -	return 0;
> +	return do_cmd(pid,&req_header, (char *)&sock_path_msg,
> +		&res_header, NULL);
>   }
>
>   /**
> @@ -554,60 +641,30 @@ int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
>    */
>   int ustcmd_get_sock_path(char **sock_path, pid_t pid)
>   {
> -	char *cmd, *reply;
>   	int result;
> +	struct ustcomm_header req_header, res_header;
> +	struct ustcomm_sock_path *sock_path_msg;
>
> -	if (asprintf(&cmd, "%s", "get_sock_path")<  0) {
> -		ERR("ustcmd_get_sock_path : asprintf failed");
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = GET_SOCK_PATH;
> +	req_header.size = 0;
>
> -	result = ustcmd_send_cmd(cmd, pid,&reply);
> -	if (result != 1) {
> -		free(cmd);
> -		return USTCMD_ERR_GEN;
> +	result = do_cmd(pid,&req_header, NULL,&res_header,
> +			(char **)&sock_path_msg);
> +	if (result<  0) {
> +		return -1;
>   	}
>
> -	free(cmd);
> -	*sock_path = reply;
> +	*sock_path = sock_path_msg->data;
>   	return 0;
>   }
>
>   int ustcmd_force_switch(pid_t pid)
>   {
> -	int result;
> +	struct ustcomm_header req_header, res_header;
>
> -	result = ustcmd_send_cmd("force_switch", pid, NULL);
> -	if (result != 1) {
> -		return USTCMD_ERR_GEN;
> -	}
> +	req_header.command = FORCE_SUBBUF_SWITCH;
> +	req_header.size = 0;
>
> -	return 0;
> +	return do_cmd(pid,&req_header, NULL,&res_header, NULL);
>   }
>
> -/**
> - * Sends a given command to a traceable process
> - *
> - * @param cmd	Null-terminated command to send
> - * @param pid	Targeted PID
> - * @param reply	Pointer to string to be filled with a reply string (must
> - *		be NULL if no reply is needed for the given command).
> - * @return	-1 if not successful, 0 on EOT, 1 on success
> - */
> -
> -int ustcmd_send_cmd(const char *cmd, const pid_t pid, char **reply)
> -{
> -	int app_fd;
> -	int retval;
> -
> -	if (ustcomm_connect_app(pid,&app_fd)) {
> -		ERR("could not connect to PID %u", (unsigned int) pid);
> -		return -1;
> -	}
> -
> -	retval = ustcomm_send_request(app_fd, cmd, reply);
> -
> -	close(app_fd);
> -
> -	return retval;
> -}
> diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
> index 50038cb..4b949be 100644
> --- a/libustcomm/ustcomm.c
> +++ b/libustcomm/ustcomm.c
> @@ -277,58 +277,58 @@ del_sock:
>   	ustcomm_del_sock(sock, keep_socket_file);
>   }
>
> +int ustcomm_recv_alloc(int sock,
> +		       struct ustcomm_header *header,
> +		       char **data) {
> +	int result;
> +	struct ustcomm_header peek_header;
> +	struct iovec iov[2];
> +	struct msghdr msg;
>
> -/* Called by an app to ask the consumer daemon to connect to it. */
> -
> -int ustcomm_request_consumer(pid_t pid, const char *channel)
> -{
> -	int result, daemon_fd;
> -	int retval = 0;
> -	char *msg=NULL;
> -	char *explicit_daemon_socket_path, *daemon_path;
> +	/* Just to make the caller fail hard */
> +	*data = NULL;
>
> -	explicit_daemon_socket_path = getenv("UST_DAEMON_SOCKET");
> -	if (explicit_daemon_socket_path) {
> -		/* user specified explicitly a socket path */
> -		result = asprintf(&daemon_path, "%s", explicit_daemon_socket_path);
> -	} else {
> -		/* just use the default path */
> -		result = asprintf(&daemon_path, "%s/ustd", SOCK_DIR);
> -	}
> -	if (result<  0) {
> -		ERR("string overflow allocating socket name");
> -		return -1;
> +	result = recv(sock,&peek_header, sizeof(peek_header),
> +		      MSG_PEEK | MSG_WAITALL);
> +	if (result<= 0) {
> +		if(errno == ECONNRESET) {
> +			return 0;
> +		} else if (errno == EINTR) {
> +			return -1;
> +		} else if (result<  0) {
> +			PERROR("recv");
> +			return -1;
> +		}
> +		return 0;
>   	}
>
> -	if (asprintf(&msg, "collect %d %s", pid, channel)<  0) {
> -		ERR("ustcomm_request_consumer : asprintf failed (collect %d/%s)",
> -		    pid, channel);
> -		retval = -1;
> -		goto free_daemon_path;
> -	}
> +	memset(&msg, 0, sizeof(msg));
>
> -	result = ustcomm_connect_path(daemon_path,&daemon_fd);
> -	if (result<  0) {
> -		WARN("ustcomm_connect_path failed, daemon_path: %s",
> -		     daemon_path);
> -		retval = -1;
> -		goto del_string;
> +	iov[0].iov_base = (char *)header;
> +	iov[0].iov_len = sizeof(struct ustcomm_header);
> +
> +	msg.msg_iov = iov;
> +	msg.msg_iovlen = 1;
> +
> +	if (peek_header.size) {
> +		*data = malloc(peek_header.size);

zmalloc

> +		if (!*data) {
> +			return -ENOMEM;
> +		}
> +
> +		iov[1].iov_base = *data;
> +		iov[1].iov_len = peek_header.size;
> +
> +		msg.msg_iovlen++;
>   	}
>
> -	result = ustcomm_send_request(daemon_fd, msg, NULL);
> +	result = recvmsg(sock,&msg, MSG_WAITALL);
>   	if (result<  0) {
> -		WARN("ustcomm_send_request failed, daemon path: %s",
> -		     daemon_path);
> -		retval = -1;
> +		free(*data);
> +		PERROR("recvmsg failed");
>   	}
>
> -	close(daemon_fd);
> -del_string:
> -	free(msg);
> -free_daemon_path:
> -	free(daemon_path);
> -
> -	return retval;
> +	return result;
>   }
>
>   /* returns 1 to indicate a message was received
> @@ -337,10 +337,9 @@ free_daemon_path:
>    */
>   int ustcomm_recv_fd(int sock,
>   		    struct ustcomm_header *header,
> -		    char **data, int *fd)
> +		    char *data, int *fd)
>   {
>   	int result;
> -	int retval;
>   	struct ustcomm_header peek_header;
>   	struct iovec iov[2];
>   	struct msghdr msg;
> @@ -369,16 +368,14 @@ int ustcomm_recv_fd(int sock,
>   	msg.msg_iov = iov;
>   	msg.msg_iovlen = 1;
>
> -	if (peek_header.size) {
> -		if (peek_header.size<  0 || peek_header.size>  100) {
> -			WARN("big peek header! %d", peek_header.size);
> -		}
> -		*data = malloc(peek_header.size);
> -		if (!*data) {
> -			ERR("failed to allocate space for message");
> +	if (peek_header.size&&  data) {
> +		if (peek_header.size<  0 ||
> +		    peek_header.size>  USTCOMM_DATA_SIZE) {
> +			ERR("big peek header! %d", peek_header.size);
> +			return 0;
>   		}
>
> -		iov[1].iov_base = (char *)*data;
> +		iov[1].iov_base = data;
>   		iov[1].iov_len = peek_header.size;
>
>   		msg.msg_iovlen++;
> @@ -389,22 +386,12 @@ int ustcomm_recv_fd(int sock,
>   		msg.msg_controllen = sizeof(buf);
>   	}
>
> -	result = recvmsg(sock,&msg,
> -			 MSG_WAITALL);
> -
> +	result = recvmsg(sock,&msg, MSG_WAITALL);
>   	if (result<= 0) {
> -		if(errno == ECONNRESET) {
> -			retval = 0;
> -		} else if (errno == EINTR) {
> -			retval = -1;
> -		} else if (result<  0) {
> -			PERROR("recv");
> -			retval = -1;
> -		} else {
> -			retval = 0;
> +		if (result<  0) {
> +			PERROR("recvmsg failed");
>   		}
> -		free(*data);
> -		return retval;
> +		return result;
>   	}
>
>   	if (fd&&  peek_header.fd_included) {
> @@ -429,19 +416,12 @@ int ustcomm_recv_fd(int sock,
>
>   int ustcomm_recv(int sock,
>   		 struct ustcomm_header *header,
> -		 char **data)
> +		 char *data)
>   {
>   	return ustcomm_recv_fd(sock, header, data, NULL);
>   }
>
>
> -int recv_message_conn(int sock, char **msg)
> -{
> -	struct ustcomm_header header;
> -
> -	return ustcomm_recv(sock,&header, msg);
> -}
> -
>   int ustcomm_send_fd(int sock,
>   		    const struct ustcomm_header *header,
>   		    const char *data,
> @@ -461,7 +441,7 @@ int ustcomm_send_fd(int sock,
>   	msg.msg_iov = iov;
>   	msg.msg_iovlen = 1;
>
> -	if (header->size) {
> +	if (header->size&&  data) {
>   		iov[1].iov_base = (char *)data;
>   		iov[1].iov_len = header->size;
>
> @@ -494,68 +474,20 @@ int ustcomm_send(int sock,
>   	return ustcomm_send_fd(sock, header, data, NULL);
>   }
>
> -int ustcomm_send_reply(char *msg, int sock)
> -{
> -	int result;
> -	struct ustcomm_header header;
> -
> -	memset(&header, 0, sizeof(header));
> -
> -	header.size = strlen(msg) + 1;
> -
> -	result = ustcomm_send(sock,&header, msg);
> -	if(result<  0) {
> -		ERR("error in ustcomm_send");
> -		return result;
> -	}
> -
> -	return 0;
> -}
> -
> -int ustcomm_send_req(int sock,
> -		     const struct ustcomm_header *req_header,
> -		     const char *data,
> -		     char **response)
> +int ustcomm_req(int sock,
> +		const struct ustcomm_header *req_header,
> +		const char *req_data,
> +		struct ustcomm_header *res_header,
> +		char *res_data)
>   {
>   	int result;
> -	struct ustcomm_header res_header;
>
> -	result = ustcomm_send(sock, req_header, data);
> +	result = ustcomm_send(sock, req_header, req_data);
>   	if ( result<= 0) {
>   		return result;
>   	}
>
> -	if (!response) {
> -		return 1;
> -	}
> -
> -	return ustcomm_recv(sock,
> -			&res_header,
> -			    response);
> -
> -}
> -
> -/*
> - * Return value:
> - *   0: Success, but no reply because recv() returned 0
> - *   1: Success
> - *   -1: Error
> - *
> - * On error, the error message is printed, except on
> - * ECONNRESET, which is normal when the application dies.
> - */
> -
> -int ustcomm_send_request(int sock, const char *req, char **reply)
> -{
> -	struct ustcomm_header req_header;
> -
> -	req_header.size = strlen(req) + 1;
> -
> -	return ustcomm_send_req(sock,
> -				&req_header,
> -				req,
> -				reply);
> -
> +	return ustcomm_recv(sock, res_header, res_data);
>   }
>
>   /* Return value:
> @@ -659,95 +591,202 @@ int ensure_dir_exists(const char *dir)
>   	return 0;
>   }
>
> -/* Used by the daemon to initialize its server so applications
> - * can connect to it.
> - */
> +char * ustcomm_print_data(char *data_field, int field_size,
> +			  int *offset, const char *format, ...)
> +{
> +	va_list args;
> +	int count, limit;
> +	char *ptr = USTCOMM_POISON_PTR;
>
> +	limit = field_size - *offset;
> +	va_start(args, format);
> +	count = vsnprintf(&data_field[*offset], limit, format, args);
> +	va_end(args);
>
> -static const char *find_tok(const char *str)
> -{
> -	while(*str == ' ') {
> -		str++;
> +	if (count<  limit&&  count>  -1) {
> +		ptr = NULL + *offset;
> +		*offset = *offset + count + 1;
> +	}
> +
> +	return ptr;
> +}
>
> -		if(*str == 0)
> -			return NULL;
> +char * ustcomm_restore_ptr(char *ptr, char *data_field, int data_field_size)
> +{
> +	if ((unsigned long)ptr>  data_field_size ||
> +	    ptr == USTCOMM_POISON_PTR) {
> +		return NULL;
>   	}
>
> -	return str;
> +	return data_field + (long)ptr;
>   }
>
> -static const char *find_sep(const char *str)
> +
> +int ustcomm_pack_channel_info(struct ustcomm_header *header,
> +			      struct ustcomm_channel_info *ch_inf,
> +			      const char *channel)
>   {
> -	while(*str != ' ') {
> -		str++;
> +	int offset = 0;
>
> -		if(*str == 0)
> -			break;
> +	ch_inf->channel = ustcomm_print_data(ch_inf->data,
> +					     sizeof(ch_inf->data),
> +					&offset,
> +					     channel);
> +
> +	if (ch_inf->channel == USTCOMM_POISON_PTR) {
> +		return -ENOMEM;
>   	}
>
> -	return str;
> +	header->size = COMPUTE_MSG_SIZE(ch_inf, offset);
> +
> +	return 0;
>   }
>
> -int nth_token_is(const char *str, const char *token, int tok_no)
> +
> +int ustcomm_unpack_channel_info(struct ustcomm_channel_info *ch_inf)
>   {
> -	int i;
> -	const char *start;
> -	const char *end;
> +	ch_inf->channel = ustcomm_restore_ptr(ch_inf->channel,
> +					      ch_inf->data,
> +					      sizeof(ch_inf->data));
> +	if (!ch_inf->channel) {
> +		return -EINVAL;
> +	}
>
> -	for(i=0; i<=tok_no; i++) {
> -		str = find_tok(str);
> -		if(str == NULL)
> -			return -1;
> +	return 0;
> +}
>
> -		start = str;
> +int ustcomm_pack_buffer_info(struct ustcomm_header *header,
> +			     struct ustcomm_buffer_info *buf_inf,
> +			     const char *channel,
> +			     int channel_cpu)
> +{
> +	int offset = 0;
>
> -		str = find_sep(str);
> -		if(str == NULL)
> -			return -1;
> +	buf_inf->channel = ustcomm_print_data(buf_inf->data,
> +					      sizeof(buf_inf->data),
> +					&offset,
> +					      channel);
>
> -		end = str;
> +	if (buf_inf->channel == USTCOMM_POISON_PTR) {
> +		return -ENOMEM;
>   	}
>
> -	if(end-start != strlen(token))
> -		return 0;
> +	buf_inf->ch_cpu = channel_cpu;
>
> -	if(strncmp(start, token, end-start))
> -		return 0;
> +	header->size = COMPUTE_MSG_SIZE(buf_inf, offset);
>
> -	return 1;
> +	return 0;
>   }
>
> -char *nth_token(const char *str, int tok_no)
> +
> +int ustcomm_unpack_buffer_info(struct ustcomm_buffer_info *buf_inf)
>   {
> -	static char *retval = NULL;
> -	int i;
> -	const char *start;
> -	const char *end;
> +	buf_inf->channel = ustcomm_restore_ptr(buf_inf->channel,
> +					       buf_inf->data,
> +					       sizeof(buf_inf->data));
> +	if (!buf_inf->channel) {
> +		return -EINVAL;
> +	}
>
> -	for(i=0; i<=tok_no; i++) {
> -		str = find_tok(str);
> -		if(str == NULL)
> -			return NULL;
> +	return 0;
> +}
>
> -		start = str;
> +int ustcomm_pack_marker_info(struct ustcomm_header *header,
> +			     struct ustcomm_marker_info *marker_inf,
> +			     const char *channel,
> +			     const char *marker)
> +{
> +	int offset = 0;
>
> -		str = find_sep(str);
> -		if(str == NULL)
> -			return NULL;
> +	marker_inf->channel = ustcomm_print_data(marker_inf->data,
> +						 sizeof(marker_inf->data),
> +						&offset,
> +						 channel);
>
> -		end = str;
> +	if (marker_inf->channel == USTCOMM_POISON_PTR) {
> +		return -ENOMEM;
>   	}
>
> -	if(retval) {
> -		free(retval);
> -		retval = NULL;
> +
> +	marker_inf->marker = ustcomm_print_data(marker_inf->data,
> +						 sizeof(marker_inf->data),
> +						&offset,
> +						 marker);
> +
> +	if (marker_inf->marker == USTCOMM_POISON_PTR) {
> +		return -ENOMEM;
>   	}
>
> -	if (asprintf(&retval, "%.*s", (int)(end-start), start)<  0) {
> -		ERR("nth_token : asprintf failed (%.*s)",
> -		    (int)(end-start), start);
> -		return NULL;
> +	header->size = COMPUTE_MSG_SIZE(marker_inf, offset);
> +
> +	return 0;
> +}
> +
> +int ustcomm_unpack_marker_info(struct ustcomm_marker_info *marker_inf)
> +{
> +	marker_inf->channel = ustcomm_restore_ptr(marker_inf->channel,
> +						  marker_inf->data,
> +						  sizeof(marker_inf->data));
> +	if (!marker_inf->channel) {
> +		return -EINVAL;
>   	}
>
> -	return retval;
> +	marker_inf->marker = ustcomm_restore_ptr(marker_inf->marker,
> +						 marker_inf->data,
> +						 sizeof(marker_inf->data));
> +	if (!marker_inf->marker) {
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +int ustcomm_send_ch_req(int sock, char *channel, int command,
> +			struct ustcomm_header *recv_header,
> +			char *recv_data)
> +{
> +	struct ustcomm_header send_header;
> +	struct ustcomm_channel_info ch_info;
> +	int result;
> +
> +	result = ustcomm_pack_channel_info(&send_header,
> +					&ch_info,
> +					   channel);
> +	if (result<  0) {
> +		return result;
> +	}
> +
> +	send_header.command = command;
> +
> +	return ustcomm_req(sock,
> +			&send_header,
> +			   (char *)&ch_info,
> +			   recv_header,
> +			   recv_data);
> +}
> +
> +int ustcomm_send_buf_req(int sock, char *channel, int ch_cpu,
> +			 int command,
> +			 struct ustcomm_header *recv_header,
> +			 char *recv_data)
> +{
> +	struct ustcomm_header send_header;
> +	struct ustcomm_buffer_info buf_info;
> +	int result;
> +
> +	result = ustcomm_pack_buffer_info(&send_header,
> +					&buf_info,
> +					  channel,
> +					  ch_cpu);
> +	if (result<  0) {
> +		return result;
> +	}
> +
> +	send_header.command = command;
> +
> +	return ustcomm_req(sock,
> +			&send_header,
> +			   (char *)&buf_info,
> +			   recv_header,
> +			   recv_data);
>   }
> diff --git a/libustcomm/ustcomm.h b/libustcomm/ustcomm.h
> index f3c07b6..9367d6e 100644
> --- a/libustcomm/ustcomm.h
> +++ b/libustcomm/ustcomm.h
> @@ -25,7 +25,6 @@
>   #include<ust/kcompat/kcompat.h>
>
>   #define SOCK_DIR "/tmp/ust-app-socks"
> -#define UST_SIGNAL SIGIO
>
>   struct ustcomm_sock {
>   	struct list_head list;
> @@ -34,15 +33,86 @@ struct ustcomm_sock {
>   };
>
>   struct ustcomm_header {
> -	int type;
> -	long size;
>   	int command;
> -	int response;
> +	long size;
> +	int result;
>   	int fd_included;
>   };
>
> +#define USTCOMM_BUFFER_SIZE ((1<<  12) - sizeof(struct ustcomm_header))
> +
> +/* Specify a sata size that leaves margin at the end of a buffer
> + * in order to make sure that we never have more data than
> + * will fit in the buffer AND that the last chars (due to a
> + * pre-receive memset) will always be 0, terminating any string
> + */
> +#define USTCOMM_DATA_SIZE (USTCOMM_BUFFER_SIZE - 20 * sizeof(void *))
> +

Would it be a real problem to sort these command in the enum ? :P

In case you don't know (Just in case :P)
(vim : CTRL+v (select first 3 carac) , hit <enter>, :sort) :D

> +enum tracectl_commands {
> +	PRINT_MARKERS,
> +	LIST_MARKERS,
> +	PRINT_TRACE_EVENTS,
> +	LIST_TRACE_EVENTS,
> +	START,
> +	CREATE_TRACE,
> +	SETUP_TRACE,
> +	ALLOC_TRACE,
> +	START_TRACE,
> +	STOP_TRACE,
> +	DESTROY_TRACE,
> +	GET_BUF_SHMID_PIPE_FD,
> +	GET_SUBBUF_NUM_SIZE,
> +	LOAD_PROBE_LIB,
> +	GET_SUBBUFFER,
> +	NOTIFY_BUF_MAPPED,
> +	PUT_SUBBUFFER,
> +	SET_SUBBUF_SIZE,
> +	SET_SUBBUF_NUM,
> +	ENABLE_MARKER,
> +	DISABLE_MARKER,
> +	GET_PIDUNIQUE,
> +	GET_SOCK_PATH,
> +	SET_SOCK_PATH,
> +	FORCE_SUBBUF_SWITCH,
> +	CONSUME_BUFFER,
> +	EXIT
> +};
> +
> +struct ustcomm_channel_info {
> +	char *channel;
> +	unsigned int subbuf_size;
> +	unsigned int subbuf_num;
> +	char data[USTCOMM_DATA_SIZE];
> +};
>
> -//int send_message_pid(pid_t pid, const char *msg, char **reply);
> +struct ustcomm_buffer_info {
> +	char *channel;
> +	int ch_cpu;
> +	pid_t pid;
> +	int buf_shmid;
> +	int buf_struct_shmid;
> +	long consumed_old;
> +	char data[USTCOMM_DATA_SIZE];
> +};
> +
> +struct ustcomm_marker_info {
> +	char *channel;
> +	char *marker;
> +	char data[USTCOMM_DATA_SIZE];
> +};
> +
> +struct ustcomm_sock_path {
> +	char *sock_path;
> +	char data[USTCOMM_DATA_SIZE];
> +};
> +
> +struct ustcomm_pidunique {
> +	s64 pidunique;
> +};
> +
> +struct ustcomm_notify_buf_mapped {
> +	char data[USTCOMM_DATA_SIZE];
> +};
>
>   /* Ensure directory existence, usefull for unix sockets */
>   extern int ensure_dir_exists(const char *dir);
> @@ -62,23 +132,74 @@ extern void ustcomm_del_named_sock(struct ustcomm_sock *sock,
>   extern int ustcomm_send_fd(int sock, const struct ustcomm_header *header,
>   			   const char *data, int *fd);
>   extern int ustcomm_recv_fd(int sock, struct ustcomm_header *header,
> -			   char **data, int *fd);
> +			   char *data, int *fd);
>
>   /* Normal send and receive functions */
>   extern int ustcomm_send(int sock, const struct ustcomm_header *header,
>   			const char *data);
>   extern int ustcomm_recv(int sock, struct ustcomm_header *header,
> -			char **data);
> +			char *data);
> +
> +/* Receive and allocate data, not to be used inside libust */
> +extern int ustcomm_recv_alloc(int sock,
> +			      struct ustcomm_header *header,
> +			      char **data);
>
> +/* Request function, send and receive */
> +extern int ustcomm_req(int sock,
> +		       const struct ustcomm_header *req_header,
> +		       const char *req_data,
> +		       struct ustcomm_header *res_header,
> +		       char *res_data);
>
>   extern int ustcomm_request_consumer(pid_t pid, const char *channel);
>   extern int ustcomm_connect_app(pid_t pid, int *app_fd);
>   extern int ustcomm_connect_path(const char *path, int *connection_fd);
> -extern int ustcomm_send_request(int sock, const char *req, char **reply);
> -extern int ustcomm_send_reply(char *msg, int sock);
> -extern int recv_message_conn(int sock, char **msg);
> +
>   extern int nth_token_is(const char *str, const char *token, int tok_no);
>
>   extern char *nth_token(const char *str, int tok_no);
>
> +/* String serialising functions, printf straight into a buffer */
> +#define USTCOMM_POISON_PTR (void *)0x19831018
> +
> +extern char * ustcomm_print_data(char *data_field, int field_size,
> +				 int *offset, const char *format, ...);
> +extern char * ustcomm_restore_ptr(char *ptr, char *data_field,
> +				  int data_field_size);
> +
> +#define COMPUTE_MSG_SIZE(struct_ptr, offset)				\
> +	(size_t) (long)(struct_ptr)->data - (long)(struct_ptr) + (offset)
> +
> +/* Packing and unpacking functions, making life easier */
> +extern int ustcomm_pack_channel_info(struct ustcomm_header *header,
> +				     struct ustcomm_channel_info *ch_inf,
> +				     const char *channel);
> +
> +extern int ustcomm_unpack_channel_info(struct ustcomm_channel_info *ch_inf);
> +
> +extern int ustcomm_pack_buffer_info(struct ustcomm_header *header,
> +				    struct ustcomm_buffer_info *buf_inf,
> +				    const char *channel,
> +				    int channel_cpu);
> +
> +extern int ustcomm_unpack_buffer_info(struct ustcomm_buffer_info *buf_inf);
> +
> +extern int ustcomm_pack_marker_info(struct ustcomm_header *header,
> +				    struct ustcomm_marker_info *marker_inf,
> +				    const char *channel,
> +				    const char *marker);
> +
> +extern int ustcomm_unpack_marker_info(struct ustcomm_marker_info *marker_inf);
> +
> +/* Packing and requesting functions */
> +extern int ustcomm_send_ch_req(int sock, char *channel, int command,
> +			       struct ustcomm_header *recv_header,
> +			       char *recv_data);
> +
> +extern int ustcomm_send_buf_req(int sock, char *channel, int ch_cpu,
> +				int command,
> +				struct ustcomm_header *recv_header,
> +				char *recv_data);
> +
>   #endif /* USTCOMM_H */
> diff --git a/libustd/libustd.c b/libustd/libustd.c
> index 5cc2108..07b37fb 100644
> --- a/libustd/libustd.c
> +++ b/libustd/libustd.c
> @@ -37,11 +37,6 @@
>   #include "usterr.h"
>   #include "ustcomm.h"
>
> -/* return value: 0 = subbuffer is finished, it won't produce data anymore
> - *               1 = got subbuffer successfully
> - *<0 = error
> - */
> -
>   #define GET_SUBBUF_OK 1
>   #define GET_SUBBUF_DONE 0
>   #define GET_SUBBUF_DIED 2
> @@ -53,134 +48,94 @@
>
>   #define UNIX_PATH_MAX 108

Since we are here chuping everything :P, this define, I have absolutely 
NO IDEA why it's set to 108... linux/limits.h has a PATH_MAX defined at 
4096...

Nothing more for the moments... This is quite huge...!!

I'll do some torture test on UST now because I've reached my "brain 
limits" on code reading :P

However, very good job for this part! Pretty cool stuff

Thanks
David

>
> -int get_subbuffer(struct buffer_info *buf)
> +static int get_subbuffer(struct buffer_info *buf)
>   {
> -	char *send_msg=NULL;
> -	char *received_msg=NULL;
> -	char *rep_code=NULL;
> -	int retval;
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_buffer_info _send_msg, _recv_msg;
> +	struct ustcomm_buffer_info *send_msg, *recv_msg;
>   	int result;
>
> -	if (asprintf(&send_msg, "get_subbuffer %s", buf->name)<  0) {
> -		ERR("get_subbuffer : asprintf failed (%s)",
> -		    buf->name);
> -		retval = -1;
> -		goto end;
> -	}
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	send_msg =&_send_msg;
> +	recv_msg =&_recv_msg;
>
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	if((result == -1&&  (errno == ECONNRESET || errno == EPIPE)) || result == 0) {
> -		DBG("app died while being traced");
> -		retval = GET_SUBBUF_DIED;
> -		goto end;
> -	}
> -	else if(result<  0) {
> -		ERR("get_subbuffer: ustcomm_send_request failed");
> -		retval = -1;
> -		goto end;
> +	result = ustcomm_pack_buffer_info(send_hdr, send_msg,
> +					  buf->channel, buf->channel_cpu);
> +	if (result<  0) {
> +		return result;
>   	}
>
> -	result = sscanf(received_msg, "%as %ld",&rep_code,&buf->consumed_old);
> -	if(result != 2&&  result != 1) {
> -		ERR("unable to parse response to get_subbuffer");
> -		retval = -1;
> -		free(received_msg);
> -		goto end_rep;
> +	send_hdr->command = GET_SUBBUFFER;
> +
> +	result = ustcomm_req(buf->app_sock, send_hdr, (char *)send_msg,
> +			     recv_hdr, (char *)recv_msg);
> +	if ((result<  0&&  (errno == ECONNRESET || errno == EPIPE)) ||
> +	    result == 0) {
> +		DBG("app died while being traced");
> +		return GET_SUBBUF_DIED;
> +	} else if (result<  0) {
> +		ERR("get_subbuffer: ustcomm_req failed");
> +		return result;
>   	}
>
> -	if (!strcmp(rep_code, "OK")) {
> +	if (!recv_hdr->result) {
>   		DBG("got subbuffer %s", buf->name);
> -		retval = GET_SUBBUF_OK;
> -	} else if(!strcmp(received_msg, "NOTFOUND")) {
> -		DBG("For buffer %s, the trace was not found. This likely means it was destroyed by the user.", buf->name);
> -		retval = GET_SUBBUF_DIED;
> -		goto end_rep;
> -	} else {
> -		DBG("error getting subbuffer %s", buf->name);
> -		retval = -1;
> +		buf->consumed_old = recv_msg->consumed_old;
> +		return GET_SUBBUF_OK;
> +	} else if (recv_hdr->result == -ENODATA) {
> +		DBG("For buffer %s, the trace was not found. This likely means"
> +		    " it was destroyed by the user.", buf->name);
> +		return GET_SUBBUF_DIED;
>   	}
>
> -	/* FIXME: free correctly the stuff */
> -end_rep:
> -	if(rep_code)
> -		free(rep_code);
> -end:
> -	if(send_msg)
> -		free(send_msg);
> -	if(received_msg)
> -		free(received_msg);
> -
> -	return retval;
> +	DBG("error getting subbuffer %s", buf->name);
> +	return recv_hdr->result;
>   }
>
> -int put_subbuffer(struct buffer_info *buf)
> +static int put_subbuffer(struct buffer_info *buf)
>   {
> -	char *send_msg=NULL;
> -	char *received_msg=NULL;
> -	char *rep_code=NULL;
> -	int retval;
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_buffer_info _send_msg, *send_msg;
>   	int result;
>
> -	if (asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old)<  0) {
> -		ERR("put_subbuffer : asprintf failed (%s %ld)",
> -		    buf->name, buf->consumed_old);
> -		retval = -1;
> -		goto end;
> -	}
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	if(result<  0&&  (errno == ECONNRESET || errno == EPIPE)) {
> -		retval = PUT_SUBBUF_DIED;
> -		goto end;
> -	}
> -	else if(result<  0) {
> -		ERR("put_subbuffer: send_message failed");
> -		retval = -1;
> -		goto end;
> -	}
> -	else if(result == 0) {
> -		/* Program seems finished. However this might not be
> -		 * the last subbuffer that has to be collected.
> -		 */
> -		retval = PUT_SUBBUF_DIED;
> -		goto end;
> -	}
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	send_msg =&_send_msg;
>
> -	result = sscanf(received_msg, "%as",&rep_code);
> -	if(result != 1) {
> -		ERR("unable to parse response to put_subbuffer");
> -		retval = -1;
> -		goto end_rep;
> +	result = ustcomm_pack_buffer_info(send_hdr, send_msg,
> +					  buf->channel, buf->channel_cpu);
> +	if (result<  0) {
> +		return result;
>   	}
>
> -	if(!strcmp(rep_code, "OK")) {
> -		DBG("subbuffer put %s", buf->name);
> -		retval = PUT_SUBBUF_OK;
> -	}
> -	else if(!strcmp(received_msg, "NOTFOUND")) {
> -		DBG("For buffer %s, the trace was not found. This likely means it was destroyed by the user.", buf->name);
> -		/* However, maybe this was not the last subbuffer. So
> -		 * we return the program died.
> -		 */
> -		retval = PUT_SUBBUF_DIED;
> -		goto end_rep;
> -	}
> -	else {
> -		DBG("put_subbuffer: received error, we were pushed");
> -		retval = PUT_SUBBUF_PUSHED;
> -		goto end_rep;
> -	}
> +	send_hdr->command = PUT_SUBBUFFER;
> +	send_msg->consumed_old = buf->consumed_old;
>
> -end_rep:
> -	if(rep_code)
> -		free(rep_code);
> +	result = ustcomm_req(buf->app_sock, send_hdr, (char *)send_msg,
> +			     recv_hdr, NULL);
> +	if ((result<  0&&  (errno == ECONNRESET || errno == EPIPE)) ||
> +	    result == 0) {
> +		DBG("app died while being traced");
> +		return PUT_SUBBUF_DIED;
> +	} else if (result<  0) {
> +		ERR("put_subbuffer: ustcomm_req failed");
> +		return result;
> +	}
>
> -end:
> -	if(send_msg)
> -		free(send_msg);
> -	if(received_msg)
> -		free(received_msg);
> +	if (!recv_hdr->result) {
> +		DBG("put subbuffer %s", buf->name);
> +		return PUT_SUBBUF_OK;
> +	} else if (recv_hdr->result == -ENODATA) {
> +		DBG("For buffer %s, the trace was not found. This likely means"
> +		    " it was destroyed by the user.", buf->name);
> +		return PUT_SUBBUF_DIED;
> +	}
>
> -	return retval;
> +	DBG("error getting subbuffer %s", buf->name);
> +	return recv_hdr->result;
>   }
>
>   void decrement_active_buffers(void *arg)
> @@ -191,139 +146,219 @@ void decrement_active_buffers(void *arg)
>   	pthread_mutex_unlock(&instance->mutex);
>   }
>
> -struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid, const char *bufname)
> +static int get_pidunique(int sock, s64 *pidunique)
>   {
> -	struct buffer_info *buf;
> -	char *send_msg;
> -	char *received_msg;
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_pidunique _recv_msg, *recv_msg;
>   	int result;
> -	struct shmid_ds shmds;
> -	struct ustcomm_header header;
>
> -	buf = (struct buffer_info *) zmalloc(sizeof(struct buffer_info));
> -	if(buf == NULL) {
> -		ERR("add_buffer: insufficient memory");
> -		return NULL;
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	recv_msg =&_recv_msg;
> +
> +	memset(send_hdr, 0, sizeof(*send_hdr));
> +
> +	send_hdr->command = GET_PIDUNIQUE;
> +	result = ustcomm_req(sock, send_hdr, NULL, recv_hdr, (char *)recv_msg);
> +	if (result<  1) {
> +		return -ENOTCONN;
> +	}
> +	if (recv_hdr->result<  0) {
> +		ERR("App responded with error: %s", strerror(recv_hdr->result));
> +		return recv_hdr->result;
>   	}
>
> -	buf->name = bufname;
> -	buf->pid = pid;
> +	*pidunique = recv_msg->pidunique;
>
> -	/* FIXME: Fix all the freeing and exit sequence from this functions */
> -	/* connect to app */
> -	result = ustcomm_connect_app(buf->pid,&buf->app_sock);
> -	if(result) {
> -		WARN("unable to connect to process, it probably died before we were able to connect");
> -		return NULL;
> +	return 0;
> +}
> +
> +static int get_buf_shmid_pipe_fd(int sock, struct buffer_info *buf,
> +				 int *buf_shmid, int *buf_struct_shmid,
> +				 int *buf_pipe_fd)
> +{
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_buffer_info _send_msg, *send_msg;
> +	struct ustcomm_buffer_info _recv_msg, *recv_msg;
> +	int result, recv_pipe_fd;
> +
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	send_msg =&_send_msg;
> +	recv_msg =&_recv_msg;
> +
> +	result = ustcomm_pack_buffer_info(send_hdr, send_msg,
> +					  buf->channel, buf->channel_cpu);
> +	if (result<  0) {
> +		ERR("Failed to pack buffer info");
> +		return result;
>   	}
>
> -	/* get pidunique */
> -	if (asprintf(&send_msg, "get_pidunique")<  0) {
> -		ERR("connect_buffer : asprintf failed (get_pidunique)");
> -		return NULL;
> +	send_hdr->command = GET_BUF_SHMID_PIPE_FD;
> +
> +	result = ustcomm_send(sock, send_hdr, (char *)send_msg);
> +	if (result<  1) {
> +		ERR("Failed to send request");
> +		return -ENOTCONN;
>   	}
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	free(send_msg);
> -	if(result == -1) {
> -		ERR("problem in ustcomm_send_request(get_pidunique)");
> -		return NULL;
> +	result = ustcomm_recv_fd(sock, recv_hdr, (char *)recv_msg,&recv_pipe_fd);
> +	if (result<  1) {
> +		ERR("Failed to receive message and fd");
> +		return -ENOTCONN;
>   	}
> -	if(result == 0) {
> -		goto error;
> +	if (recv_hdr->result<  0) {
> +		ERR("App responded with error %s", strerror(recv_hdr->result));
> +		return recv_hdr->result;
>   	}
>
> -	result = sscanf(received_msg, "%lld",&buf->pidunique);
> -	if(result != 1) {
> -		ERR("unable to parse response to get_pidunique");
> -		return NULL;
> -	}
> -	free(received_msg);
> -	DBG("got pidunique %lld", buf->pidunique);
> +	*buf_shmid = recv_msg->buf_shmid;
> +	*buf_struct_shmid = recv_msg->buf_struct_shmid;
> +	*buf_pipe_fd = recv_pipe_fd;
>
> -	/* get shmid */
> -	if (asprintf(&send_msg, "get_shmid %s", buf->name)<  0) {
> -		ERR("connect_buffer : asprintf failed (get_schmid %s)",
> -		    buf->name);
> -		return NULL;
> -	}
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	free(send_msg);
> -	if(result == -1) {
> -		ERR("problem in ustcomm_send_request(get_shmid)");
> -		return NULL;
> +	return 0;
> +}
> +
> +static int get_subbuf_num_size(int sock, struct buffer_info *buf,
> +			       int *subbuf_num, int *subbuf_size)
> +{
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_channel_info _send_msg, *send_msg;
> +	struct ustcomm_channel_info _recv_msg, *recv_msg;
> +	int result;
> +
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	send_msg =&_send_msg;
> +	recv_msg =&_recv_msg;
> +
> +	result = ustcomm_pack_channel_info(send_hdr, send_msg,
> +					   buf->channel);
> +	if (result<  0) {
> +		return result;
>   	}
> -	if(result == 0) {
> -		goto error;
> +
> +	send_hdr->command = GET_SUBBUF_NUM_SIZE;
> +
> +	result = ustcomm_req(sock, send_hdr, (char *)send_msg,
> +			     recv_hdr, (char *)recv_msg);
> +	if (result<  1) {
> +		return -ENOTCONN;
>   	}
>
> -	result = sscanf(received_msg, "%d %d",&buf->shmid,&buf->bufstruct_shmid);
> -	if(result != 2) {
> -		ERR("unable to parse response to get_shmid (\"%s\")", received_msg);
> -		return NULL;
> +	*subbuf_num = recv_msg->subbuf_num;
> +	*subbuf_size = recv_msg->subbuf_size;
> +
> +	return recv_hdr->result;
> +}
> +
> +
> +static int notify_buffer_mapped(int sock, struct buffer_info *buf)
> +{
> +	struct ustcomm_header _send_hdr, *send_hdr;
> +	struct ustcomm_header _recv_hdr, *recv_hdr;
> +	struct ustcomm_buffer_info _send_msg, *send_msg;
> +	int result;
> +
> +	send_hdr =&_send_hdr;
> +	recv_hdr =&_recv_hdr;
> +	send_msg =&_send_msg;
> +
> +	result = ustcomm_pack_buffer_info(send_hdr, send_msg,
> +					  buf->channel, buf->channel_cpu);
> +	if (result<  0) {
> +		return result;
>   	}
> -	free(received_msg);
> -	DBG("got shmids %d %d", buf->shmid, buf->bufstruct_shmid);
>
> -	/* get n_subbufs */
> -	if (asprintf(&send_msg, "get_n_subbufs %s", buf->name)<  0) {
> -		ERR("connect_buffer : asprintf failed (get_n_subbufs %s)",
> -		    buf->name);
> -		return NULL;
> +	send_hdr->command = NOTIFY_BUF_MAPPED;
> +
> +	result = ustcomm_req(sock, send_hdr, (char *)send_msg,
> +			     recv_hdr, NULL);
> +	if (result<  1) {
> +		return -ENOTCONN;
>   	}
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	free(send_msg);
> -	if(result == -1) {
> -		ERR("problem in ustcomm_send_request(g_n_subbufs)");
> +
> +	return recv_hdr->result;
> +}
> +
> +
> +struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
> +				   const char *channel, int channel_cpu)
> +{
> +	struct buffer_info *buf;
> +	int result;
> +	struct shmid_ds shmds;
> +
> +	buf = (struct buffer_info *) zmalloc(sizeof(struct buffer_info));
> +	if(buf == NULL) {
> +		ERR("add_buffer: insufficient memory");
>   		return NULL;
>   	}
> -	if(result == 0) {
> -		goto error;
> -	}
>
> -	result = sscanf(received_msg, "%d",&buf->n_subbufs);
> -	if(result != 1) {
> -		ERR("unable to parse response to get_n_subbufs");
> -		return NULL;
> +	buf->channel = strdup(channel);
> +	if (!buf->channel) {
> +		goto free_buf;
>   	}
> -	free(received_msg);
> -	DBG("got n_subbufs %d", buf->n_subbufs);
>
> -	/* get subbuf size */
> -	if (asprintf(&send_msg, "get_subbuf_size %s", buf->name)<  0) {
> -		ERR("connect_buffer : asprintf failed (get_subbuf_size %s)",
> -		    buf->name);
> -		return NULL;
> +	result = asprintf(&buf->name, "%s_%d", channel, channel_cpu);
> +	if (result<  0 || buf->name == NULL) {
> +		goto free_buf_channel;
>   	}
> -	result = ustcomm_send_request(buf->app_sock, send_msg,&received_msg);
> -	free(send_msg);
> -	if(result == -1) {
> -		ERR("problem in ustcomm_send_request(get_subbuf_size)");
> -		return NULL;
> +
> +	buf->channel_cpu = channel_cpu;
> +	buf->pid = pid;
> +
> +	result = ustcomm_connect_app(buf->pid,&buf->app_sock);
> +	if(result) {
> +		WARN("unable to connect to process, it probably died before we were able to connect");
> +		goto free_buf_name;
>   	}
> -	if(result == 0) {
> -		goto error;
> +
> +	/* get pidunique */
> +	result = get_pidunique(buf->app_sock,&buf->pidunique);
> +	if (result<  0) {
> +		ERR("Failed to get pidunique");
> +		goto close_app_sock;
> +	}
> +
> +	/* get shmid and pipe fd */
> +	result = get_buf_shmid_pipe_fd(buf->app_sock, buf,&buf->shmid,
> +				&buf->bufstruct_shmid,&buf->pipe_fd);
> +	if (result<  0) {
> +		ERR("Failed to get buf_shmid and pipe_fd");
> +		goto close_app_sock;
> +	} else {
> +		struct stat temp;
> +		fstat(buf->pipe_fd,&temp);
> +		if (!S_ISFIFO(temp.st_mode)) {
> +			ERR("Didn't receive a fifo from the app");
> +			goto close_app_sock;
> +		}
>   	}
>
> -	result = sscanf(received_msg, "%d",&buf->subbuf_size);
> -	if(result != 1) {
> -		ERR("unable to parse response to get_subbuf_size");
> -		return NULL;
> +
> +	/* get number of subbufs and subbuf size */
> +	result = get_subbuf_num_size(buf->app_sock, buf,&buf->n_subbufs,
> +				&buf->subbuf_size);
> +	if (result<  0) {
> +		ERR("Failed to get subbuf number and size");
> +		goto close_fifo;
>   	}
> -	free(received_msg);
> -	DBG("got subbuf_size %d", buf->subbuf_size);
>
>   	/* attach memory */
>   	buf->mem = shmat(buf->shmid, NULL, 0);
>   	if(buf->mem == (void *) 0) {
>   		PERROR("shmat");
> -		return NULL;
> +		goto close_fifo;
>   	}
>   	DBG("successfully attached buffer memory");
>
>   	buf->bufstruct_mem = shmat(buf->bufstruct_shmid, NULL, 0);
>   	if(buf->bufstruct_mem == (void *) 0) {
>   		PERROR("shmat");
> -		return NULL;
> +		goto shmdt_mem;
>   	}
>   	DBG("successfully attached buffer bufstruct memory");
>
> @@ -331,36 +366,16 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>   	result = shmctl(buf->shmid, IPC_STAT,&shmds);
>   	if(result == -1) {
>   		PERROR("shmctl");
> -		return NULL;
> +		goto shmdt_bufstruct_mem;
>   	}
>   	buf->memlen = shmds.shm_segsz;
>
> -	/* get buffer pipe fd */
> -	memset(&header, 0, sizeof(header));
> -	if (asprintf(&send_msg, "get_buffer_fd %s", buf->name)<  0) {
> -		ERR("connect_buffer : asprintf failed (get_buffer_fd %s)",
> -		    buf->name);
> -		return NULL;
> -	}
> -	header.size = strlen(send_msg) + 1;
> -	result = ustcomm_send(buf->app_sock,&header, send_msg);
> -	free(send_msg);
> -	if (result<= 0) {
> -		ERR("ustcomm_send failed.");
> -		return NULL;
> -	}
> -	result = ustcomm_recv_fd(buf->app_sock,&header, NULL,&buf->pipe_fd);
> -	if (result<= 0) {
> -		ERR("ustcomm_recv_fd failed");
> -		return NULL;
> -	} else {
> -		struct stat temp;
> -		fstat(buf->pipe_fd,&temp);
> -		if (!S_ISFIFO(temp.st_mode)) {
> -			ERR("Didn't receive a fifo from the app");
> -			return NULL;
> -		}
> +	/* Notify the application that we have mapped the buffer */
> +	result = notify_buffer_mapped(buf->app_sock, buf);
> +	if (result<  0) {
> +		goto shmdt_bufstruct_mem;
>   	}
> +
>   	if(instance->callbacks->on_open_buffer)
>   		instance->callbacks->on_open_buffer(instance->callbacks, buf);
>
> @@ -370,7 +385,25 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>
>   	return buf;
>
> -error:
> +shmdt_bufstruct_mem:
> +	shmdt(buf->bufstruct_mem);
> +
> +shmdt_mem:
> +	shmdt(buf->mem);
> +
> +close_fifo:
> +	close(buf->pipe_fd);
> +
> +close_app_sock:
> +	close(buf->app_sock);
> +
> +free_buf_name:
> +	free(buf->name);
> +
> +free_buf_channel:
> +	free(buf->channel);
> +
> +free_buf:
>   	free(buf);
>   	return NULL;
>   }
> @@ -413,7 +446,7 @@ int consumer_loop(struct libustd_instance *instance, struct buffer_info *buf)
>   		/* get the subbuffer */
>   		if (read_result == 1) {
>   			result = get_subbuffer(buf);
> -			if(result == -1) {
> +			if (result<  0) {
>   				ERR("error getting subbuffer");
>   				continue;
>   			} else if (result == GET_SUBBUF_DIED) {
> @@ -475,7 +508,8 @@ int consumer_loop(struct libustd_instance *instance, struct buffer_info *buf)
>
>   struct consumer_thread_args {
>   	pid_t pid;
> -	const char *bufname;
> +	const char *channel;
> +	int channel_cpu;
>   	struct libustd_instance *instance;
>   };
>
> @@ -486,8 +520,6 @@ void *consumer_thread(void *arg)
>   	int result;
>   	sigset_t sigset;
>
> -	DBG("GOT ARGS: pid %d bufname %s", args->pid, args->bufname);
> -
>   	if(args->instance->callbacks->on_new_thread)
>   		args->instance->callbacks->on_new_thread(args->instance->callbacks);
>
> @@ -513,7 +545,8 @@ void *consumer_thread(void *arg)
>   		goto end;
>   	}
>
> -	buf = connect_buffer(args->instance, args->pid, args->bufname);
> +	buf = connect_buffer(args->instance, args->pid,
> +			     args->channel, args->channel_cpu);
>   	if(buf == NULL) {
>   		ERR("failed to connect to buffer");
>   		goto end;
> @@ -528,26 +561,32 @@ void *consumer_thread(void *arg)
>   	if(args->instance->callbacks->on_close_thread)
>   		args->instance->callbacks->on_close_thread(args->instance->callbacks);
>
> -	free((void *)args->bufname);
> +	free((void *)args->channel);
>   	free(args);
>   	return NULL;
>   }
>
> -int start_consuming_buffer(
> -	struct libustd_instance *instance, pid_t pid, const char *bufname)
> +int start_consuming_buffer(struct libustd_instance *instance, pid_t pid,
> +			   const char *channel, int channel_cpu)
>   {
>   	pthread_t thr;
>   	struct consumer_thread_args *args;
>   	int result;
>
> -	DBG("beginning of start_consuming_buffer: args: pid %d bufname %s", pid, bufname);
> +	DBG("beginning of start_consuming_buffer: args: pid %d bufname %s_%d", pid, channel,
> +	    channel_cpu);
>
>   	args = (struct consumer_thread_args *) zmalloc(sizeof(struct consumer_thread_args));
> +	if (!args) {
> +		return -ENOMEM;
> +	}
>
>   	args->pid = pid;
> -	args->bufname = strdup(bufname);
> +	args->channel = strdup(channel);
> +	args->channel_cpu = channel_cpu;
>   	args->instance = instance;
> -	DBG("beginning2 of start_consuming_buffer: args: pid %d bufname %s", args->pid, args->bufname);
> +	DBG("beginning2 of start_consuming_buffer: args: pid %d bufname %s_%d",
> +	    args->pid, args->channel, args->channel_cpu);
>
>   	result = pthread_create(&thr, NULL, consumer_thread, args);
>   	if(result == -1) {
> @@ -559,37 +598,54 @@ int start_consuming_buffer(
>   		ERR("pthread_detach failed");
>   		return -1;
>   	}
> -	DBG("end of start_consuming_buffer: args: pid %d bufname %s", args->pid, args->bufname);
> +	DBG("end of start_consuming_buffer: args: pid %d bufname %s_%d",
> +	    args->pid, args->channel, args->channel_cpu);
>
>   	return 0;
>   }
> -static void process_client_cmd(char *recvbuf, struct libustd_instance *instance)
> +static void process_client_cmd(int sock, struct ustcomm_header *req_header,
> +			       char *recvbuf, struct libustd_instance *instance)
>   {
> -	if(!strncmp(recvbuf, "collect", 7)) {
> -		pid_t pid;
> -		char *bufname = NULL;
> -		int result;
> +	int result;
> +	struct ustcomm_header _res_header;
> +	struct ustcomm_header *res_header =&_res_header;
> +	struct ustcomm_buffer_info *buf_inf;
> +
> +	DBG("Processing client command");
>
> -		result = sscanf(recvbuf, "%*s %d %50as",&pid,&bufname);
> -		if (result != 2) {
> -			ERR("parsing error: %s", recvbuf);
> -			goto free_bufname;
> +	switch (req_header->command) {
> +	case CONSUME_BUFFER:
> +
> +		buf_inf = (struct ustcomm_buffer_info *)recvbuf;
> +		result = ustcomm_unpack_buffer_info(buf_inf);
> +		if (result<  0) {
> +			ERR("Couldn't unpack buffer info");
> +			return;
>   		}
>
> -		result = start_consuming_buffer(instance, pid, bufname);
> +		DBG("Going to consume buffer %s_%d in process %d",
> +		    buf_inf->channel, buf_inf->ch_cpu, buf_inf->pid);
> +		result = start_consuming_buffer(instance, buf_inf->pid,
> +						buf_inf->channel,
> +						buf_inf->ch_cpu);
>   		if (result<  0) {
>   			ERR("error in add_buffer");
> -			goto free_bufname;
> +			return;
>   		}
>
> -	free_bufname:
> -		if (bufname) {
> -			free(bufname);
> -		}
> -	} else if(!strncmp(recvbuf, "exit", 4)) {
> +		res_header->result = 0;
> +		break;
> +	case EXIT:
> +		res_header->result = 0;
>   		/* Only there to force poll to return */
> -	} else {
> -		WARN("unknown command: %s", recvbuf);
> +		break;
> +	default:
> +		res_header->result = -EINVAL;
> +		WARN("unknown command: %d", req_header->command);
> +	}
> +
> +	if (ustcomm_send(sock, res_header, NULL)<= 0) {
> +		ERR("couldn't send command response");
>   	}
>   }
>
> @@ -597,6 +653,8 @@ static void process_client_cmd(char *recvbuf, struct libustd_instance *instance)
>
>   int libustd_start_instance(struct libustd_instance *instance)
>   {
> +	struct ustcomm_header recv_hdr;
> +	char recv_buf[USTCOMM_BUFFER_SIZE];
>   	struct ustcomm_sock *epoll_sock;
>   	struct epoll_event events[MAX_EVENTS];
>   	struct sockaddr addr;
> @@ -635,13 +693,14 @@ int libustd_start_instance(struct libustd_instance *instance)
>   				ustcomm_init_sock(accept_fd, epoll_fd,
>   						&instance->connections);
>   			} else {
> -				char *msg = NULL;
> -				result = recv_message_conn(epoll_sock->fd,&msg);
> -				if (result == 0) {
> +				result = ustcomm_recv(epoll_sock->fd,&recv_hdr,
> +						      recv_buf);
> +				if (result<  1) {
>   					ustcomm_del_sock(epoll_sock, 0);
> -				} else if (msg) {
> -					process_client_cmd(msg, instance);
> -					free(msg);
> +				} else {
> +					process_client_cmd(epoll_sock->fd,
> +							&recv_hdr, recv_buf,
> +							   instance);
>   				}
>
>   			}
> diff --git a/ustctl/ustctl.c b/ustctl/ustctl.c
> index bf149d1..07630dc 100644
> --- a/ustctl/ustctl.c
> +++ b/ustctl/ustctl.c
> @@ -169,6 +169,59 @@ int parse_opts_long(int argc, char **argv, struct ust_opts *opts)
>   	return 0;
>   }
>
> +static int scan_ch_marker(const char *channel_marker, char **channel,
> +			char **marker)
> +{
> +	int result;
> +
> +	*channel = NULL;
> +	*marker = NULL;
> +
> +	result = sscanf(channel_marker, "%a[^/]/%as", channel, marker);
> +	if (result == 0) {
> +		ERR("Failed to parse marker and channel names, got EOF");
> +		return -1;
> +	} else if (result<  0) {
> +		PERROR("Failed to parse marker and channel names");
> +		return -1;
> +	} else if (result != 2) {
> +		ERR("Failed to parse marker and channel names");
> +		if (channel) {
> +			free(channel);
> +		}
> +		if (marker) {
> +			free(marker);
> +		}
> +		return -1;
> +	} else if (result == 2) {
> +		return 0;
> +	}
> +}
> +
> +static int scan_ch_and_num(const char *ch_num, char **channel, unsigned int *num)
> +{
> +	int result;
> +
> +	*channel = NULL;
> +
> +	result = sscanf(ch_num, "%a[^/]/%u", channel, num);
> +	if (result == 0) {
> +		ERR("Failed to parse channel and number, got EOF");
> +		return -1;
> +	} else if (result<  0) {
> +		PERROR("Failed to parse channel and number");
> +		return -1;
> +	} else if (result != 2) {
> +		ERR("Failed to parse channel and number");
> +		if (channel) {
> +			free(channel);
> +		}
> +		return -1;
> +	} else if (result == 2) {
> +		return 0;
> +	}
> +}
> +
>   int main(int argc, char *argv[])
>   {
>   	pid_t *pidit;
> @@ -301,16 +354,31 @@ int main(int argc, char *argv[])
>   				break;
>   			case ENABLE_MARKER:
>   				if (opts.regex) {
> -					if (ustcmd_set_marker_state(opts.regex, 1, *pidit)) {
> -						ERR("error while trying to enable marker %s with PID %u\n",
> -								opts.regex, (unsigned int) *pidit);
> +					char *channel, *marker;
> +
> +					if (scan_ch_marker(opts.regex,
> +							&channel,&marker)) {
> +						retval = EXIT_FAILURE;
> +						break;
> +					}
> +					if (ustcmd_set_marker_state(channel, marker, 1, *pidit)) {
> +						PERROR("error while trying to enable marker %s with PID %u",
> +						       opts.regex, (unsigned int) *pidit);
>   						retval = EXIT_FAILURE;
>   					}
>   				}
> +
>   				break;
>   			case DISABLE_MARKER:
>   				if (opts.regex) {
> -					if (ustcmd_set_marker_state(opts.regex, 0, *pidit)) {
> +					char *channel, *marker;
> +
> +					if (scan_ch_marker(opts.regex,
> +							&channel,&marker)) {
> +						retval = EXIT_FAILURE;
> +						break;
> +					}
> +					if (ustcmd_set_marker_state(channel, marker, 0, *pidit)) {
>   						ERR("error while trying to disable marker %s with PID %u\n",
>   								opts.regex, (unsigned int) *pidit);
>   						retval = EXIT_FAILURE;
> @@ -320,7 +388,14 @@ int main(int argc, char *argv[])
>
>   			case SET_SUBBUF_SIZE:
>   				if (opts.regex) {
> -					if (ustcmd_set_subbuf_size(opts.regex, *pidit)) {
> +					char *channel;
> +					unsigned int size;
> +					if (scan_ch_and_num(opts.regex,&channel,&size)) {
> +						retval = EXIT_FAILURE;
> +						break;
> +					}
> +
> +					if (ustcmd_set_subbuf_size(channel, size, *pidit)) {
>   						ERR("error while trying to set the size of subbuffers with PID %u\n",
>   								(unsigned int) *pidit);
>   						retval = EXIT_FAILURE;
> @@ -330,7 +405,19 @@ int main(int argc, char *argv[])
>
>   			case SET_SUBBUF_NUM:
>   				if (opts.regex) {
> -					if (ustcmd_set_subbuf_num(opts.regex, *pidit)) {
> +					char *channel;
> +					unsigned int num;
> +					if (scan_ch_and_num(opts.regex,&channel,&num)) {
> +						retval = EXIT_FAILURE;
> +						break;
> +					}
> +
> +					if (num<= 2) {
> +						ERR("Subbuffer count should be greater than 2");
> +						retval = EXIT_FAILURE;
> +						break;
> +					}
> +					if (ustcmd_set_subbuf_num(channel, num, *pidit)) {
>   						ERR("error while trying to set the number of subbuffers with PID %u\n",
>   								(unsigned int) *pidit);
>   						retval = EXIT_FAILURE;

-- 
David Goulet
LTTng project, DORSAL Lab.

PGP/GPG : 1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563




More information about the lttng-dev mailing list