{"thread":{"id":"47548","subject":"[PATCH] upload-pack: fix some sparse warnings","startedAt":"2018-01-05T20:31:07Z","lastAt":"2018-01-08T18:51:11Z","messageCount":2,"participants":["Ramsay Jones","Brandon Williams"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"336001","messageId":"720013d4-a493-e0b6-7a38-9ebbbaa198ce@ramsayjones.plus.com","threadId":"47548","inReplyTo":null,"subject":"[PATCH] upload-pack: fix some sparse warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-01-05T20:30:59Z","receivedAt":"2018-01-05T20:31:07Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Brandon,\n\nIf you need to re-roll your 'bw/protocol-v2' branch, could you please\nsquash this (or something like it) into the relevant patches. The first\nhunk would go in commit 6ec1105192, \"upload-pack: convert to a builtin\",\n2018-01-02), whereas the second hunk would go to commit b3f3749a24,\n\"upload-pack: factor out processing lines\", 2018-01-02).\n\nThe sparse warnings were:\n\n SP upload-pack.c\n upload-pack.c:783:43: error: incompatible types for operation (<=)\n upload-pack.c:783:43: left side has type int *depth\n upload-pack.c:783:43: right side has type int\n upload-pack.c:783:43: error: incorrect type in conditional\n upload-pack.c:783:43: got bad type\n upload-pack.c:1389:5: warning: symbol 'cmd_upload_pack' was not declared. Should it be static?\n\n[Note that the line numbers are off-by-one.]\n\nI note, in passing, that strtol() returns a 'long int' but *depth is\nan 'int' (yes, the original code was like that too ;-) ).\n\nShould cmd_upload_pack(), along with the #include \"builtin.h\", be moved\nto builtin/upload-pack.c?\n\nAlso, I note that packet_read_with_status(), introduced in commit 4570421c3,\nis not called outside of pkt-line.c; does this symbol need to be extern?\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n upload-pack.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 8002f1f20..6271245e2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"builtin.h\"\n #include \"config.h\"\n #include \"refs.h\"\n #include \"pkt-line.h\"\n@@ -780,7 +781,7 @@ static int process_deepen(const char *line, int *depth)\n \tif (skip_prefix(line, \"deepen \", &arg)) {\n \t\tchar *end = NULL;\n \t\t*depth = strtol(arg, &end, 0);\n-\t\tif (!end || *end || depth <= 0)\n+\t\tif (!end || *end || *depth <= 0)\n \t\t\tdie(\"Invalid deepen: %s\", line);\n \t\treturn 1;\n \t}\n-- \n2.15.0\n"},{"id":"336169","messageId":"20180108185103.GA156560@google.com","threadId":"47548","inReplyTo":"720013d4-a493-e0b6-7a38-9ebbbaa198ce@ramsayjones.plus.com","subject":"Re: [PATCH] upload-pack: fix some sparse warnings","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-01-08T18:51:03Z","receivedAt":"2018-01-08T18:51:11Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 01/05, Ramsay Jones wrote:\n> \n> Signed-off-by: Ramsay Jones \n> ---\n> \n> Hi Brandon,\n> \n> If you need to re-roll your 'bw/protocol-v2' branch, could you please\n> squash this (or something like it) into the relevant patches. The first\n> hunk would go in commit 6ec1105192, \"upload-pack: convert to a builtin\",\n> 2018-01-02), whereas the second hunk would go to commit b3f3749a24,\n> \"upload-pack: factor out processing lines\", 2018-01-02).\n\nThanks for finding these, I'll make sure I include them in the relevant\ncommits.\n\n> \n> The sparse warnings were:\n> \n> SP upload-pack.c\n> upload-pack.c:783:43: error: incompatible types for operation (<=)\n> upload-pack.c:783:43: left side has type int *depth\n> upload-pack.c:783:43: right side has type int\n> upload-pack.c:783:43: error: incorrect type in conditional\n> upload-pack.c:783:43: got bad type\n> upload-pack.c:1389:5: warning: symbol 'cmd_upload_pack' was not declared. Should it be static?\n> \n> [Note that the line numbers are off-by-one.]\n> \n> I note, in passing, that strtol() returns a 'long int' but *depth is\n> an 'int' (yes, the original code was like that too ;-) ).\n> \n> Should cmd_upload_pack(), along with the #include \"builtin.h\", be moved\n> to builtin/upload-pack.c?\n\nJunio mentioned something similar when I sent out the WIP series, I must\nhave forgotten to do that before sending this out. I'll make that\nchange :)\n\n> \n> Also, I note that packet_read_with_status(), introduced in commit 4570421c3,\n> is not called outside of pkt-line.c; does this symbol need to be extern?\n\nI thought it might, but you're right it doesn't look like it needs to.\nI'll change that so its not exported.\n\n> \n> Thanks!\n> \n> ATB,\n> Ramsay Jones\n> \n> \n> upload-pack.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/upload-pack.c b/upload-pack.c\n> index 8002f1f20..6271245e2 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -1,4 +1,5 @@\n> #include \"cache.h\"\n> +#include \"builtin.h\"\n> #include \"config.h\"\n> #include \"refs.h\"\n> #include \"pkt-line.h\"\n> @@ -780,7 +781,7 @@ static int process_deepen(const char *line, int *depth)\n> \tif (skip_prefix(line, \"deepen \", &arg)) {\n> \t\tchar *end = NULL;\n> \t\t*depth = strtol(arg, &end, 0);\n> -\t\tif (!end || *end || depth <= 0)\n> +\t\tif (!end || *end || *depth <= 0)\n> \t\t\tdie(\"Invalid deepen: %s\", line);\n> \t\treturn 1;\n> \t}\n> -- \n> 2.15.0\n\n-- \nBrandon Williams\n"}]}