{"thread":{"id":"28941","subject":"[PATCH 0/4] Fix minor warnings reported by icc","startedAt":"2011-11-15T16:59:35Z","lastAt":"2011-11-16T07:49:07Z","messageCount":9,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"179510","messageId":"1321376379-31750-1-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":null,"subject":"[PATCH 0/4] Fix minor warnings reported by icc","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T16:59:35Z","receivedAt":"2011-11-15T16:59:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThomas Rast tried compiling Git with Intel's C compiler (icc) recently\nand the results can be found here [1].  Most of them don't amount to\nanything, are probably not worth fixing.  Here are fixes to a few that\ncaught my eye.\n\nThanks.\n\n[1]: https://gist.github.com/1367335\n\n-- Ram\n\nRamkumar Ramachandra (4):\n  http: remove unused function hex()\n  convert: don't mix enum with int\n  ll-merge: initialize default_opts const\n  sha1_file: don't mix enum with int\n\n convert.c   |    6 +++---\n http.c      |    8 --------\n ll-merge.c  |    2 +-\n sha1_file.c |    2 +-\n 4 files changed, 5 insertions(+), 13 deletions(-)\n\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179512","messageId":"1321376379-31750-2-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":"1321376379-31750-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/4] http: remove unused function hex()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T16:59:36Z","receivedAt":"2011-11-15T16:59:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n http.c |    8 --------\n 1 files changed, 0 insertions(+), 8 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 008ad72..e6c7597 100644\n--- a/http.c\n+++ b/http.c\n@@ -747,14 +747,6 @@ static inline int needs_quote(int ch)\n \treturn 1;\n }\n \n-static inline int hex(int v)\n-{\n-\tif (v < 10)\n-\t\treturn '0' + v;\n-\telse\n-\t\treturn 'A' + v - 10;\n-}\n-\n static char *quote_ref_url(const char *base, const char *ref)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179511","messageId":"1321376379-31750-3-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":"1321376379-31750-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/4] convert: don't mix enum with int","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T16:59:37Z","receivedAt":"2011-11-15T16:59:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n convert.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 3bb5a4d..038b0be 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -641,7 +641,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,\n \treturn 1;\n }\n \n-static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n+static enum crlf_action git_path_check_crlf(const char *path, struct git_attr_check *check)\n {\n \tconst char *value = check->value;\n \n@@ -658,7 +658,7 @@ static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n \treturn CRLF_GUESS;\n }\n \n-static int git_path_check_eol(const char *path, struct git_attr_check *check)\n+static enum crlf_action git_path_check_eol(const char *path, struct git_attr_check *check)\n {\n \tconst char *value = check->value;\n \n@@ -811,7 +811,7 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\treturn ret | convert_to_git(path, src, len, dst, 0);\n+\treturn ret | convert_to_git(path, src, len, dst, SAFE_CRLF_FALSE);\n }\n \n /*****************************************************************\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179513","messageId":"1321376379-31750-4-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":"1321376379-31750-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/4] ll-merge: initialize default_opts const","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T16:59:38Z","receivedAt":"2011-11-15T16:59:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n ll-merge.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/ll-merge.c b/ll-merge.c\nindex da59738..205aed3 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     const struct ll_merge_options *opts)\n {\n \tstatic struct git_attr_check check[2];\n-\tstatic const struct ll_merge_options default_opts;\n+\tstatic const struct ll_merge_options default_opts = {0, 0, 0, 0};\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n \tconst struct ll_merge_driver *driver;\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179514","messageId":"1321376379-31750-5-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":"1321376379-31750-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/4] sha1_file: don't mix enum with int","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T16:59:39Z","receivedAt":"2011-11-15T16:59:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sha1_file.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 27f3b9b..869852b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2616,7 +2616,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \tif ((type == OBJ_BLOB) && path) {\n \t\tstruct strbuf nbuf = STRBUF_INIT;\n \t\tif (convert_to_git(path, buf, size, &nbuf,\n-\t\t                   write_object ? safe_crlf : 0)) {\n+\t\t                   write_object ? safe_crlf : SAFE_CRLF_FALSE)) {\n \t\t\tbuf = strbuf_detach(&nbuf, &size);\n \t\t\tre_allocated = 1;\n \t\t}\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179515","messageId":"1321378269-13867-1-git-send-email-artagnon@gmail.com","threadId":"28941","inReplyTo":"1321376379-31750-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/4] git-compat-util: don't assume value for undefined variable","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T17:31:09Z","receivedAt":"2011-11-15T17:31:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Suggested-by: Thomas Rast <trast@student.ethz.ch>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n And here's another probably worth fixing.\n\n git-compat-util.h |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 5ef8ff7..8b4dd5c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -219,7 +219,7 @@ extern char *gitbasename(char *);\n #define find_last_dir_sep(path) strrchr(path, '/')\n #endif\n \n-#if __HP_cc >= 61000\n+#if defined(__HP_cc) && (__HP_cc >= 61000)\n #define NORETURN __attribute__((noreturn))\n #define NORETURN_PTR\n #elif defined(__GNUC__) && !defined(NO_NORETURN)\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179539","messageId":"7vty65t1qp.fsf@alter.siamese.dyndns.org","threadId":"28941","inReplyTo":"1321376379-31750-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/4] ll-merge: initialize default_opts const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-15T23:17:02Z","receivedAt":"2011-11-15T23:17:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  ll-merge.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/ll-merge.c b/ll-merge.c\n> index da59738..205aed3 100644\n> --- a/ll-merge.c\n> +++ b/ll-merge.c\n> @@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,\n>  \t     const struct ll_merge_options *opts)\n>  {\n>  \tstatic struct git_attr_check check[2];\n> -\tstatic const struct ll_merge_options default_opts;\n> +\tstatic const struct ll_merge_options default_opts = {0, 0, 0, 0};\n\nDoesn't \"static\" tell us that it will be in BSS, initialized to all zero\nby definition?\n"},{"id":"179551","messageId":"CALkWK0mVxxq0345B_OJQwejpTBD=evOU_iAv39CvXv4mAi=09A@mail.gmail.com","threadId":"28941","inReplyTo":"7vty65t1qp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] ll-merge: initialize default_opts const","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-16T06:06:05Z","receivedAt":"2011-11-16T06:06:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>> [...]\n>> diff --git a/ll-merge.c b/ll-merge.c\n>> index da59738..205aed3 100644\n>> --- a/ll-merge.c\n>> +++ b/ll-merge.c\n>> @@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,\n>>            const struct ll_merge_options *opts)\n>>  {\n>>       static struct git_attr_check check[2];\n>> -     static const struct ll_merge_options default_opts;\n>> +     static const struct ll_merge_options default_opts = {0, 0, 0, 0};\n>\n> Doesn't \"static\" tell us that it will be in BSS, initialized to all zero\n> by definition?\n\nI'm uncertain about whether the C89 standard says this explicitly- icc\nis more pedantic than most mainstream compilers.  Feel free to drop\nthis part if you don't think it adds value.\n\nThanks.\n\n-- Ram\n"},{"id":"179563","messageId":"7vd3csqzgs.fsf@alter.siamese.dyndns.org","threadId":"28941","inReplyTo":"CALkWK0mVxxq0345B_OJQwejpTBD=evOU_iAv39CvXv4mAi=09A@mail.gmail.com","subject":"Re: [PATCH 3/4] ll-merge: initialize default_opts const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-16T07:49:07Z","receivedAt":"2011-11-16T07:49:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Hi Junio,\n>\n> Junio C Hamano wrote:\n>> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>>> [...]\n>>> -     static const struct ll_merge_options default_opts;\n>>> +     static const struct ll_merge_options default_opts = {0, 0, 0, 0};\n>>\n>> Doesn't \"static\" tell us that it will be in BSS, initialized to all zero\n>> by definition?\n>\n> I'm uncertain about whether the C89 standard says this explicitly- icc\n> is more pedantic than most mainstream compilers.\n\nActually I take a part of the comment back; as this is \"static const\", it\nis entirely plausible for a compiler to tell the linker to put it in rodata\ninstead of bss, to cause any attempt to modify it to segv.\n\nA datum that is implicitly initialized to all zero and not allowed to be\nmodified might appear suspect to some compiler writers (especially when\nthey are under influence ;-)), so I am not so surprised if a compiler\nissued a warning saying \"did you forget to initialize it?\".\n"}]}