Re: [PATCH 0/3] Fix code issues spotted by clang

On Sun, Nov 6, 2011 at 13:06, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Out of curiosity I also compiled git on "Sun Studio 12 Update 1" on ee6dfb2d83ba1b057943e705f707fa27e34e47f9 with this patch series applied and it emits the following warnings, quoted below along with my comments:

    "pack-write.c", line 76: warning: name redefined by pragma
redefine_extname declared static: tmpfile

Presumably this means that we've imported the function tmpfile() and Sun's CC doesn't like that we're creating a variable with that name.

    "read-cache.c", line 761: warning: statement not reached

SunCC's brain is melting on this particularly clever piece of code:

    	goto inside;
    	for (;;) {
    		if (!c)
    			return 1;
    		if (is_dir_sep(c)) {
    inside:

    "sha1_file.c", line 2453: warning: name redefined by pragma
redefine_extname declared static: tmpfile

same tempfile issue.

    "compat/../git-compat-util.h", line 4: warning: macro redefined:
_FILE_OFFSET_BITS
    "compat/../git-compat-util.h", line 4: warning: macro redefined:
_FILE_OFFSET_BITS

Seems we're clashing with this:

    $ grep -r '#define.*_FILE_OFFSET_BITS' /usr/include/
    /usr/include/sys/feature_tests.h:#define   _FILE_OFFSET_BITS   64
    /usr/include/sys/feature_tests.h:#define   _FILE_OFFSET_BITS   32

    "xdiff/xutils.c", line 194: warning: statement not reached

This was added in b97e911643341cb31e6b97029b9ffd96fc675b1d as a workaround on systems using glibc, should this even be run on Sun libc? Martin?

Another clever bit of code blowing SunCC's brain:

	if (flags & XDF_IGNORE_WHITESPACE) {
		goto skip_ws;
		while (i1 < s1 && i2 < s2) {
			if (l1[i1++] != l2[i2++])
				return 0;
		skip_ws:
			while (i1 < s1 && XDL_ISSPACE(l1[i1]))
				i1++;
			while (i2 < s2 && XDL_ISSPACE(l2[i2]))
				i2++;
		}
	}

    "fast-import.c", line 858: warning: name redefined by pragma
redefine_extname declared static: tmpfile

ditto tempfile issue.

    "builtin/fast-export.c", line 54: warning: enum type mismatch: op "="

This seems to me to be a legitimate issue we introduced in 2d8ad46919213ebbd7bb72eb5b56cca8cc3ae07f, Elijah?

We're defining an enum like this:

    static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;
    static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;

And then doing:

	if (unset || !strcmp(arg, "abort"))
		tag_of_filtered_mode = ABORT; <-- Line 54
	else if (!strcmp(arg, "drop"))
		tag_of_filtered_mode = DROP;
	else if (!strcmp(arg, "rewrite"))
		tag_of_filtered_mode = REWRITE;

Presumably that assignment should be "= ERROR".

    "builtin/index-pack.c", line 175: warning: name redefined by
pragma redefine_extname declared static: tmpfile

ditto tempfile.

    "vcs-svn/string_pool.c", line 11: warning: initializer will be
sign-extended: -1
    "vcs-svn/string_pool.c", line 81: warning: initializer will be
sign-extended: -1
    "vcs-svn/repo_tree.c", line 112: warning: initializer will be
sign-extended: -1
    "vcs-svn/repo_tree.c", line 112: warning: initializer will be
sign-extended: -1
    "test-treap.c", line 34: warning: initializer will be sign-extended: -1

All of these come down to assigning ~0 to an uint32_t variable, e.g.:

	uint32_t token = ~0;