git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] upload-pack: fix some sparse warnings
@ 2018-01-05 20:30 Ramsay Jones
  2018-01-08 18:51 ` Brandon Williams
  0 siblings, 1 reply; 2+ messages in thread
From: Ramsay Jones @ 2018-01-05 20:30 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, GIT Mailing-list


Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Brandon,

If you need to re-roll your 'bw/protocol-v2' branch, could you please
squash this (or something like it) into the relevant patches. The first
hunk would go in commit 6ec1105192, "upload-pack: convert to a builtin",
2018-01-02), whereas the second hunk would go to commit b3f3749a24,
"upload-pack: factor out processing lines", 2018-01-02).

The sparse warnings were:

       SP upload-pack.c
   upload-pack.c:783:43: error: incompatible types for operation (<=)
   upload-pack.c:783:43:    left side has type int *depth
   upload-pack.c:783:43:    right side has type int
   upload-pack.c:783:43: error: incorrect type in conditional
   upload-pack.c:783:43:    got bad type
   upload-pack.c:1389:5: warning: symbol 'cmd_upload_pack' was not declared. Should it be static?

[Note that the line numbers are off-by-one.]

I note, in passing, that strtol() returns a 'long int' but *depth is
an 'int' (yes, the original code was like that too ;-) ).

Should cmd_upload_pack(), along with the #include "builtin.h", be moved
to builtin/upload-pack.c?

Also, I note that packet_read_with_status(), introduced in commit 4570421c3,
is not called outside of pkt-line.c; does this symbol need to be extern?

Thanks!

ATB,
Ramsay Jones


 upload-pack.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/upload-pack.c b/upload-pack.c
index 8002f1f20..6271245e2 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "builtin.h"
 #include "config.h"
 #include "refs.h"
 #include "pkt-line.h"
@@ -780,7 +781,7 @@ static int process_deepen(const char *line, int *depth)
 	if (skip_prefix(line, "deepen ", &arg)) {
 		char *end = NULL;
 		*depth = strtol(arg, &end, 0);
-		if (!end || *end || depth <= 0)
+		if (!end || *end || *depth <= 0)
 			die("Invalid deepen: %s", line);
 		return 1;
 	}
-- 
2.15.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] upload-pack: fix some sparse warnings
  2018-01-05 20:30 [PATCH] upload-pack: fix some sparse warnings Ramsay Jones
@ 2018-01-08 18:51 ` Brandon Williams
  0 siblings, 0 replies; 2+ messages in thread
From: Brandon Williams @ 2018-01-08 18:51 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list

On 01/05, Ramsay Jones wrote:
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> 
> Hi Brandon,
> 
> If you need to re-roll your 'bw/protocol-v2' branch, could you please
> squash this (or something like it) into the relevant patches. The first
> hunk would go in commit 6ec1105192, "upload-pack: convert to a builtin",
> 2018-01-02), whereas the second hunk would go to commit b3f3749a24,
> "upload-pack: factor out processing lines", 2018-01-02).

Thanks for finding these, I'll make sure I include them in the relevant
commits.

> 
> The sparse warnings were:
> 
>        SP upload-pack.c
>    upload-pack.c:783:43: error: incompatible types for operation (<=)
>    upload-pack.c:783:43:    left side has type int *depth
>    upload-pack.c:783:43:    right side has type int
>    upload-pack.c:783:43: error: incorrect type in conditional
>    upload-pack.c:783:43:    got bad type
>    upload-pack.c:1389:5: warning: symbol 'cmd_upload_pack' was not declared. Should it be static?
> 
> [Note that the line numbers are off-by-one.]
> 
> I note, in passing, that strtol() returns a 'long int' but *depth is
> an 'int' (yes, the original code was like that too ;-) ).
> 
> Should cmd_upload_pack(), along with the #include "builtin.h", be moved
> to builtin/upload-pack.c?

Junio mentioned something similar when I sent out the WIP series, I must
have forgotten to do that before sending this out.  I'll make that
change :)

> 
> Also, I note that packet_read_with_status(), introduced in commit 4570421c3,
> is not called outside of pkt-line.c; does this symbol need to be extern?

I thought it might, but you're right it doesn't look like it needs to.
I'll change that so its not exported.

> 
> Thanks!
> 
> ATB,
> Ramsay Jones
> 
> 
>  upload-pack.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/upload-pack.c b/upload-pack.c
> index 8002f1f20..6271245e2 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1,4 +1,5 @@
>  #include "cache.h"
> +#include "builtin.h"
>  #include "config.h"
>  #include "refs.h"
>  #include "pkt-line.h"
> @@ -780,7 +781,7 @@ static int process_deepen(const char *line, int *depth)
>  	if (skip_prefix(line, "deepen ", &arg)) {
>  		char *end = NULL;
>  		*depth = strtol(arg, &end, 0);
> -		if (!end || *end || depth <= 0)
> +		if (!end || *end || *depth <= 0)
>  			die("Invalid deepen: %s", line);
>  		return 1;
>  	}
> -- 
> 2.15.0

-- 
Brandon Williams

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-01-08 18:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-05 20:30 [PATCH] upload-pack: fix some sparse warnings Ramsay Jones
2018-01-08 18:51 ` Brandon Williams

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git