[PATCH] pkt-line: Fix sparse errors and warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sparse issues the following error and warnings:

    pkt-line.c:209:51: warning: Using plain integer as NULL pointer
    sideband.c:41:52: warning: Using plain integer as NULL pointer
    daemon.c:615:39: warning: Using plain integer as NULL pointer
    remote-curl.c:220:75: error: incompatible types for operation (>)
    remote-curl.c:220:75:    left side has type char *
    remote-curl.c:220:75:    right side has type int
    remote-curl.c:291:53: warning: Using plain integer as NULL pointer
    remote-curl.c:408:43: warning: Using plain integer as NULL pointer
    remote-curl.c:562:47: warning: Using plain integer as NULL pointer

All of these complaints "blame" to commit 17243606 ("pkt-line: share
buffer/descriptor reading implementation", 20-02-2013).

In order to suppress the warnings, we simply replace the integer
constant 0 with NULL.

In order to suppress the error message, we simply remove the "> 0" from
the while loop controlling expression.

Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
---

Hi Jeff,

When you next re-roll your 'jk/pkt-line-cleanup' patches, could you
please squash this (or something like it) into commit 17243606
("pkt-line: share buffer/descriptor reading implementation", 20-02-2013).

Please check the resolution of the sparse error, remote-curl.c:220, since
I didn't think too deeply about it, making some assumptions ... ;-)

Note: the commit message mentions an strbuf_get_line() function, but
that is supposed to be packet_get_line(), right?

In addition, that commit adds the following code as part of function
get_packet_data():

+       /* Read up to "size" bytes from our source, whatever it is. */
+       if (src_buf && *src_buf) {
+               ret = size < *src_size ? size : *src_size;
+               memcpy(dst, *src_buf, ret);
+               *src_buf += size;
............................^^^^^
+               *src_size -= size;
+       } else {
+

This could lead to the source buffer pointer being incremented past the
"one past the end" of the buffer; ie to undefined behaviour. That use
of 'size', along with the one on the following line, should be 'ret' no?

ATB,
Ramsay Jones

 daemon.c      | 2 +-
 pkt-line.c    | 2 +-
 remote-curl.c | 8 ++++----
 sideband.c    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/daemon.c b/daemon.c
index 9a241d9..82d5bf5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -612,7 +612,7 @@ static int execute(void)
 		loginfo("Connection from %s:%s", addr, port);
 
 	alarm(init_timeout ? init_timeout : timeout);
-	pktlen = packet_read(0, NULL, 0, packet_buffer, sizeof(packet_buffer), 0);
+	pktlen = packet_read(0, NULL, NULL, packet_buffer, sizeof(packet_buffer), 0);
 	alarm(0);
 
 	len = strlen(line);
diff --git a/pkt-line.c b/pkt-line.c
index 116d5f1..2793ecb 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -206,7 +206,7 @@ static char *packet_read_line_generic(int fd,
 
 char *packet_read_line(int fd, int *len_p)
 {
-	return packet_read_line_generic(fd, NULL, 0, len_p);
+	return packet_read_line_generic(fd, NULL, NULL, len_p);
 }
 
 char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)
diff --git a/remote-curl.c b/remote-curl.c
index 3d2b194..93a09a6 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -217,7 +217,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
 		 * until a packet flush marker.  Ignore these now, but
 		 * in the future we might start to scan them.
 		 */
-		while (packet_read_line_buf(&last->buf, &last->len, NULL) > 0)
+		while (packet_read_line_buf(&last->buf, &last->len, NULL))
 			;
 
 		last->proto_git = 1;
@@ -288,7 +288,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 
 	if (!avail) {
 		rpc->initial_buffer = 0;
-		avail = packet_read(rpc->out, NULL, 0, rpc->buf, rpc->alloc, 0);
+		avail = packet_read(rpc->out, NULL, NULL, rpc->buf, rpc->alloc, 0);
 		if (!avail)
 			return 0;
 		rpc->pos = 0;
@@ -405,7 +405,7 @@ static int post_rpc(struct rpc_state *rpc)
 			break;
 		}
 
-		n = packet_read(rpc->out, 0, NULL, buf, left, 0);
+		n = packet_read(rpc->out, NULL, NULL, buf, left, 0);
 		if (!n)
 			break;
 		rpc->len += n;
@@ -559,7 +559,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
 	rpc->hdr_accept = strbuf_detach(&buf, NULL);
 
 	while (!err) {
-		int n = packet_read(rpc->out, 0, NULL, rpc->buf, rpc->alloc, 0);
+		int n = packet_read(rpc->out, NULL, NULL, rpc->buf, rpc->alloc, 0);
 		if (!n)
 			break;
 		rpc->pos = 0;
diff --git a/sideband.c b/sideband.c
index 857954c..d1125f5 100644
--- a/sideband.c
+++ b/sideband.c
@@ -38,7 +38,7 @@ int recv_sideband(const char *me, int in_stream, int out)
 
 	while (1) {
 		int band, len;
-		len = packet_read(in_stream, NULL, 0, buf + pf, LARGE_PACKET_MAX, 0);
+		len = packet_read(in_stream, NULL, NULL, buf + pf, LARGE_PACKET_MAX, 0);
 		if (len == 0)
 			break;
 		if (len < 1) {
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]