[RFC,v9,01/11] trace-cmd: Make ports unsigned int

Commit Message

Slavomir Kaslev April 2, 2019, 1:45 p.m. UTC

Switch ports data type to unsigned int since vsocket ports are 32 bit unsigned
integers and sometimes cause overflow when stored in int variables.

Signed-off-by: Slavomir Kaslev <[e-mail]>
---
 include/trace-cmd/trace-cmd.h |  2 +-
 tracecmd/trace-listen.c       | 10 +++++-----
 tracecmd/trace-msg.c          | 28 ++++++++++++++++++++--------
 3 files changed, 26 insertions(+), 14 deletions(-)

Comments

Steven Rostedt April 3, 2019, 1 p.m. UTC | #1

On Tue,  2 Apr 2019 16:45:30 +0300
Slavomir Kaslev <[e-mail]> wrote:

> -static int write_ints(char *buf, size_t buf_len, int *arr, int arr_len)
> +static unsigned int atou(const char *s)
> +{
> +	long r;
> +
> +	r = atol(s);
> +	if (r >= 0 && r <= UINT_MAX)
> +		return r;
> +
> +	return 0;
> +}
> +

I wonder if we should do this instead:

/* test a to u */
static int tatou(const char *s, int *res)
{
	long r;

	r = atol(s);
	if (r >= 0 && r <= UINT_MAX) {
		*res = (unsigned)r;
		return 0;
	}
	return -1;
}

That way we could check for invalid ints.

-- Steve

Slavomir Kaslev April 3, 2019, 1:14 p.m. UTC | #2

On Wed, 2019-04-03 at 09:00 -0400, Steven Rostedt wrote:
> On Tue,  2 Apr 2019 16:45:30 +0300
> Slavomir Kaslev <[e-mail]> wrote:
> 
> > -static int write_ints(char *buf, size_t buf_len, int *arr, int
> > arr_len)
> > +static unsigned int atou(const char *s)
> > +{
> > +	long r;
> > +
> > +	r = atol(s);
> > +	if (r >= 0 && r <= UINT_MAX)
> > +		return r;
> > +
> > +	return 0;
> > +}
> > +
> 
> I wonder if we should do this instead:
> 
> /* test a to u */
> static int tatou(const char *s, int *res)
> {
> 	long r;
> 
> 	r = atol(s);
> 	if (r >= 0 && r <= UINT_MAX) {
> 		*res = (unsigned)r;
> 		return 0;
> 	}
> 	return -1;
> }

Makes sense. I did consider it while writing this but decided to go
with the traditional "broken" prototype for such functions.

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index ca4452b..9cfb987 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -324,7 +324,7 @@  int tracecmd_msg_wait_close(struct tracecmd_msg_handle *msg_handle);
 /* for server */
 int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle);
 int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle,
-				 int *ports);
+				 unsigned *ports);
 int tracecmd_msg_read_data(struct tracecmd_msg_handle *msg_handle, int ofd);
 int tracecmd_msg_collect_data(struct tracecmd_msg_handle *msg_handle, int ofd);
 bool tracecmd_msg_done(struct tracecmd_msg_handle *msg_handle);
diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c
index 8bd7bad..ce975c8 100644
--- a/tracecmd/trace-listen.c
+++ b/tracecmd/trace-listen.c
@@ -517,10 +517,10 @@  static int *create_all_readers(const char *node, const char *port,
 {
 	int use_tcp = msg_handle->flags & TRACECMD_MSG_FL_USE_TCP;
 	char buf[BUFSIZ];
-	int *port_array;
+	unsigned int *port_array;
 	int *pid_array;
-	int start_port;
-	int udp_port;
+	unsigned int start_port;
+	unsigned int udp_port;
 	int cpus = msg_handle->cpu_count;
 	int cpu;
 	int pid;
@@ -528,11 +528,11 @@  static int *create_all_readers(const char *node, const char *port,
 	if (!pagesize)
 		return NULL;
 
-	port_array = malloc(sizeof(int) * cpus);
+	port_array = malloc(sizeof(*port_array) * cpus);
 	if (!port_array)
 		return NULL;
 
-	pid_array = malloc(sizeof(int) * cpus);
+	pid_array = malloc(sizeof(*pid_array) * cpus);
 	if (!pid_array) {
 		free(port_array);
 		return NULL;
diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index e2dd188..461b6cb 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -161,12 +161,24 @@  static int make_tinit(struct tracecmd_msg_handle *msg_handle,
 	return 0;
 }
 
-static int write_ints(char *buf, size_t buf_len, int *arr, int arr_len)
+static unsigned int atou(const char *s)
+{
+	long r;
+
+	r = atol(s);
+	if (r >= 0 && r <= UINT_MAX)
+		return r;
+
+	return 0;
+}
+
+static int write_uints(char *buf, size_t buf_len,
+		       unsigned int *arr, int arr_len)
 {
 	int i, ret, tot = 0;
 
 	for (i = 0; i < arr_len; i++) {
-		ret = snprintf(buf, buf_len, "%d", arr[i]);
+		ret = snprintf(buf, buf_len, "%u", arr[i]);
 		if (ret < 0)
 			return ret;
 
@@ -184,15 +196,15 @@  static int write_ints(char *buf, size_t buf_len, int *arr, int arr_len)
 	return tot;
 }
 
-static int make_rinit(struct tracecmd_msg *msg, int cpus, int *ports)
+static int make_rinit(struct tracecmd_msg *msg, int cpus, unsigned int *ports)
 {
 	int data_size;
 
-	data_size = write_ints(NULL, 0, ports, cpus);
+	data_size = write_uints(NULL, 0, ports, cpus);
 	msg->buf = malloc(data_size);
 	if (!msg->buf)
 		return -ENOMEM;
-	write_ints(msg->buf, data_size, ports, cpus);
+	write_uints(msg->buf, data_size, ports, cpus);
 
 	msg->rinit.cpus = htonl(cpus);
 	msg->hdr.size = htonl(ntohl(msg->hdr.size) + data_size);
@@ -442,7 +454,7 @@  int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 	}
 
 	cpus = ntohl(msg.rinit.cpus);
-	ports = malloc_or_die(sizeof(*ports) * cpus);
+	ports = malloc(sizeof(*ports) * cpus);
 	if (!ports) {
 		ret = -ENOMEM;
 		goto out;
@@ -456,7 +468,7 @@  int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 			goto error;
 		}
 
-		ports[i] = atoi(p);
+		ports[i] = atou(p);
 		p = strchr(p, '\0');
 	}
 
@@ -588,7 +600,7 @@  error:
 }
 
 int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle,
-				 int *ports)
+				 unsigned int *ports)
 {
 	struct tracecmd_msg msg;
 	int ret;