threads / patch / 28941

patch, 4 partsFix minor warnings reported by icc

Subject: [PATCH 0/4] Fix minor warnings reported by icc

## tl;dr

9 messages between Nov 15, 2011 and Nov 16, 2011. Diffs are folded; open one to read it.

replies: 8people: 2as markdown or json

Ramkumar Ramachandra· Nov 15, 2011, 16:59 UTC · lore

Hi,

Thomas Rast tried compiling Git with Intel's C compiler (icc) recently and the results can be found here [1]. Most of them don't amount to anything, are probably not worth fixing. Here are fixes to a few that caught my eye.

Thanks.

[1]: https://gist.github.com/1367335

-- Ram

Ramkumar Ramachandra (4):
  http: remove unused function hex()
  convert: don't mix enum with int
  ll-merge: initialize default_opts const
  sha1_file: don't mix enum with int

 convert.c   |    6 +++---
 http.c      |    8 --------
 ll-merge.c  |    2 +-
 sha1_file.c |    2 +-
 4 files changed, 5 insertions(+), 13 deletions(-)

-- 
1.7.6.351.gb35ac.dirty

Ramkumar Ramachandra· Nov 15, 2011, 16:59 UTC · re: Ramkumar Ramachandra · lore

[PATCH 1/4] http: remove unused function hex()

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 http.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

Show changes to http.c +0 −8

diff --git a/http.c b/http.c
index 008ad72..e6c7597 100644
--- a/http.c
+++ b/http.c
@@ -747,14 +747,6 @@ static inline int needs_quote(int ch)
 	return 1;
 }
 
-static inline int hex(int v)
-{
-	if (v < 10)
-		return '0' + v;
-	else
-		return 'A' + v - 10;
-}
-
 static char *quote_ref_url(const char *base, const char *ref)
 {
 	struct strbuf buf = STRBUF_INIT;

-- 
1.7.6.351.gb35ac.dirty

Ramkumar Ramachandra· Nov 15, 2011, 16:59 UTC · re: Ramkumar Ramachandra · lore

[PATCH 2/4] convert: don't mix enum with int

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 convert.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Show changes to convert.c +3 −3

diff --git a/convert.c b/convert.c
index 3bb5a4d..038b0be 100644
--- a/convert.c
+++ b/convert.c
@@ -641,7 +641,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
 	return 1;
 }
 
-static int git_path_check_crlf(const char *path, struct git_attr_check *check)
+static enum crlf_action git_path_check_crlf(const char *path, struct git_attr_check *check)
 {
 	const char *value = check->value;
 
@@ -658,7 +658,7 @@ static int git_path_check_crlf(const char *path, struct git_attr_check *check)
 	return CRLF_GUESS;
 }
 
-static int git_path_check_eol(const char *path, struct git_attr_check *check)
+static enum crlf_action git_path_check_eol(const char *path, struct git_attr_check *check)
 {
 	const char *value = check->value;
 
@@ -811,7 +811,7 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str
 		src = dst->buf;
 		len = dst->len;
 	}
-	return ret | convert_to_git(path, src, len, dst, 0);
+	return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_FALSE);
 }
 
 /*****************************************************************

-- 
1.7.6.351.gb35ac.dirty

Ramkumar Ramachandra· Nov 15, 2011, 16:59 UTC · re: Ramkumar Ramachandra · lore

[PATCH 3/4] ll-merge: initialize default_opts const

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 ll-merge.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to ll-merge.c +1 −1

diff --git a/ll-merge.c b/ll-merge.c
index da59738..205aed3 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,
 	     const struct ll_merge_options *opts)
 {
 	static struct git_attr_check check[2];
-	static const struct ll_merge_options default_opts;
+	static const struct ll_merge_options default_opts = {0, 0, 0, 0};
 	const char *ll_driver_name = NULL;
 	int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
 	const struct ll_merge_driver *driver;

-- 
1.7.6.351.gb35ac.dirty

Junio C Hamano· Nov 15, 2011, 23:17 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH 3/4] ll-merge: initialize default_opts const

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 15 quoted lines

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  ll-merge.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/ll-merge.c b/ll-merge.c
> index da59738..205aed3 100644
> --- a/ll-merge.c
> +++ b/ll-merge.c
> @@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,
>  	     const struct ll_merge_options *opts)
>  {
>  	static struct git_attr_check check[2];
> -	static const struct ll_merge_options default_opts;
> +	static const struct ll_merge_options default_opts = {0, 0, 0, 0};

Doesn't "static" tell us that it will be in BSS, initialized to all zero by definition?

Ramkumar Ramachandra· Nov 16, 2011, 06:06 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/4] ll-merge: initialize default_opts const

Hi Junio,

Junio C Hamano wrote:

Show 15 quoted lines

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> [...]
>> diff --git a/ll-merge.c b/ll-merge.c
>> index da59738..205aed3 100644
>> --- a/ll-merge.c
>> +++ b/ll-merge.c
>> @@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,
>>            const struct ll_merge_options *opts)
>>  {
>>       static struct git_attr_check check[2];
>> -     static const struct ll_merge_options default_opts;
>> +     static const struct ll_merge_options default_opts = {0, 0, 0, 0};
>
> Doesn't "static" tell us that it will be in BSS, initialized to all zero
> by definition?

I'm uncertain about whether the C89 standard says this explicitly- icc is more pedantic than most mainstream compilers. Feel free to drop this part if you don't think it adds value.

Thanks.

-- Ram

Junio C Hamano· Nov 16, 2011, 07:49 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH 3/4] ll-merge: initialize default_opts const

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 13 quoted lines

> Hi Junio,
>
> Junio C Hamano wrote:
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>> [...]
>>> -     static const struct ll_merge_options default_opts;
>>> +     static const struct ll_merge_options default_opts = {0, 0, 0, 0};
>>
>> Doesn't "static" tell us that it will be in BSS, initialized to all zero
>> by definition?
>
> I'm uncertain about whether the C89 standard says this explicitly- icc
> is more pedantic than most mainstream compilers.

Actually I take a part of the comment back; as this is "static const", it is entirely plausible for a compiler to tell the linker to put it in rodata instead of bss, to cause any attempt to modify it to segv.

A datum that is implicitly initialized to all zero and not allowed to be modified might appear suspect to some compiler writers (especially when they are under influence ;-)), so I am not so surprised if a compiler issued a warning saying "did you forget to initialize it?".

Ramkumar Ramachandra· Nov 15, 2011, 16:59 UTC · re: Ramkumar Ramachandra · lore

[PATCH 4/4] sha1_file: don't mix enum with int

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to sha1_file.c +1 −1

diff --git a/sha1_file.c b/sha1_file.c
index 27f3b9b..869852b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2616,7 +2616,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,
 	if ((type == OBJ_BLOB) && path) {
 		struct strbuf nbuf = STRBUF_INIT;
 		if (convert_to_git(path, buf, size, &nbuf,
-		                   write_object ? safe_crlf : 0)) {
+		                   write_object ? safe_crlf : SAFE_CRLF_FALSE)) {
 			buf = strbuf_detach(&nbuf, &size);
 			re_allocated = 1;
 		}

-- 
1.7.6.351.gb35ac.dirty

Ramkumar Ramachandra· Nov 15, 2011, 17:31 UTC · re: Ramkumar Ramachandra · lore

[PATCH 5/4] git-compat-util: don't assume value for undefined variable

Suggested-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 And here's another probably worth fixing.

 git-compat-util.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to git-compat-util.h +1 −1

diff --git a/git-compat-util.h b/git-compat-util.h
index 5ef8ff7..8b4dd5c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -219,7 +219,7 @@ extern char *gitbasename(char *);
 #define find_last_dir_sep(path) strrchr(path, '/')
 #endif
 
-#if __HP_cc >= 61000
+#if defined(__HP_cc) && (__HP_cc >= 61000)
 #define NORETURN __attribute__((noreturn))
 #define NORETURN_PTR
 #elif defined(__GNUC__) && !defined(NO_NORETURN)

-- 
1.7.6.351.gb35ac.dirty