git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	GIT Mailing-list <git@vger.kernel.org>
Subject: [PATCH] pkt-line: Fix sparse errors and warnings
Date: Sat, 23 Feb 2013 18:44:04 +0000	[thread overview]
Message-ID: <51290DF4.4040309@ramsay1.demon.co.uk> (raw)


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@ramsay1.demon.co.uk>
---

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

             reply	other threads:[~2013-02-23 18:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-23 18:44 Ramsay Jones [this message]
2013-02-23 22:31 ` [PATCH] pkt-line: Fix sparse errors and warnings Jeff King
2013-02-23 22:37   ` Jeff King
2013-02-24  8:36     ` Junio C Hamano
2013-02-26 19:02     ` Ramsay Jones
2013-02-26 18:52   ` Ramsay Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51290DF4.4040309@ramsay1.demon.co.uk \
    --to=ramsay@ramsay1.demon.co.uk \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).