{"thread":{"id":"26743","subject":"[PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","startedAt":"2011-03-16T02:49:59Z","lastAt":"2011-03-18T07:25:20Z","messageCount":34,"participants":["Jonathan Nieder","Junio C Hamano","Johannes Sixt","Nguyen Thai Ngoc Duy","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"163416","messageId":"20110316024959.GA24932@elie","threadId":"26743","inReplyTo":null,"subject":"[PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T02:49:59Z","receivedAt":"2011-03-16T02:49:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Fri, 04 Mar 2011 00:54:53 -0600\n\nStarting with gcc 4.5 (r147852, Pretty-ipa merge: Inliner heruistics\nreorg, 2009-05-25), gcc -O3 -Wall warns when building\nreflog_expire_config:\n\n warning: 'expire' may be used uninitialized in this function [-Wuninitialized]\n\nThe cause: starting with that version, gcc realizes it can inline the\ncall to parse_expire_cfg_value. In the error case, 'expire' is not\ninitialized and the function returns early, but gcc does not have\nenough information to figure out that this is an error return.\n\nSquash the warning by letting the optimizer peek at the return value\nfrom config_error_nonbool. This also decreases the text size by a\ntiny (negligible) amount when building with -Os --- before:\n\n text data bss dec hex filename\n 1002184 24856 316928 1343968 1481e0 git\n\nAfter:\n\n text data bss dec hex filename\n 1002120 24856 316928 1343904 1481a0 git\n\nSigned-off-by: Jonathan Nieder \n---\nHi,\n\nAnother patch that was sitting around in my tree. (I had the somewhat\ninsane idea of turning on as many warnings as feasible and getting git\nto build with -Werror. The effect on running time for tests was\nencouraging but within noise.)\n\nWhile it might make sense to do something like this for error()\nitself, I don't know a clean and portable way to make a variadic macro\nor inline function.\n\nAnyway, maybe this can provide some amusement.\n\n cache.h | 8 +++++++-\n config.c | 4 ++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c4ef999..c8ce53a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1022,10 +1022,16 @@ extern int check_repository_format_version(const char *var, const char *value, v\n extern int git_env_bool(const char *, int);\n extern int git_config_system(void);\n extern int git_config_global(void);\n-extern int config_error_nonbool(const char *);\n extern const char *get_log_output_encoding(void);\n extern const char *get_commit_output_encoding(void);\n \n+extern void config_print_error_nonbool(const char *);\n+static inline int config_error_nonbool(const char *var)\n+{\n+\tconfig_print_error_nonbool(var);\n+\treturn -1;\n+}\n+\n extern const char *config_exclusive_filename;\n \n #define MAX_GITNAME (1000)\ndiff --git a/config.c b/config.c\nindex b94de8f..cf41c1c 100644\n--- a/config.c\n+++ b/config.c\n@@ -1506,7 +1506,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)\n * Call this to report error for your variable that should not\n * get a boolean value (i.e. \"[my] var\" means \"true\").\n */\n-int config_error_nonbool(const char *var)\n+void config_print_error_nonbool(const char *var)\n {\n-\treturn error(\"Missing value for '%s'\", var);\n+\terror(\"Missing value for '%s'\", var);\n }\n-- \n1.7.4.1\n"},{"id":"163417","messageId":"20110316034232.GA26027@elie","threadId":"26743","inReplyTo":"20110316024959.GA24932@elie","subject":"[PATCH nd/struct-pathspec] declare 1-bit bitfields to be unsigned","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T03:42:32Z","receivedAt":"2011-03-16T03:42:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As \"gcc -pedantic\" notices, a two's complement 1-bit signed integer\ncannot represent the value '1'.\n\n dir.c: In function 'init_pathspec':\n dir.c:1291:4: warning: overflow in implicit constant conversion [-Woverflow]\n\nIn the spirit of v1.7.1-rc1~10 (2010-04-06), 'unsigned' is what was\nintended, so let's make the flags unsigned.\n\nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n\n> I had the somewhat\n> insane idea of turning on as many warnings as feasible and getting git\n> to build with -Werror.\n\nHere's another one. Based against f577b92 from master (aka\nnd/struct-pathspec).\n\n cache.h | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4beb2dc..edd5b5a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -503,13 +503,13 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct\n struct pathspec {\n \tconst char **raw; /* get_pathspec() result, not freed by free_pathspec() */\n \tint nr;\n-\tint has_wildcard:1;\n-\tint recursive:1;\n+\tunsigned int has_wildcard:1;\n+\tunsigned int recursive:1;\n \tint max_depth;\n \tstruct pathspec_item {\n \t\tconst char *match;\n \t\tint len;\n-\t\tint has_wildcard:1;\n+\t\tunsigned int has_wildcard:1;\n \t} *items;\n };\n \n-- \n1.7.4.1\n"},{"id":"163422","messageId":"7vfwqnabbi.fsf@alter.siamese.dyndns.org","threadId":"26743","inReplyTo":"20110316024959.GA24932@elie","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T05:22:57Z","receivedAt":"2011-03-16T05:22:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Date: Fri, 04 Mar 2011 00:54:53 -0600\n>\n> Starting with gcc 4.5 (r147852, Pretty-ipa merge: Inliner heruistics\n> reorg, 2009-05-25), gcc -O3 -Wall warns when building\n> reflog_expire_config:\n>\n> warning: 'expire' may be used uninitialized in this function [-Wuninitialized]\n>\n> The cause: starting with that version, gcc realizes it can inline the\n> call to parse_expire_cfg_value. In the error case, 'expire' is not\n> initialized and the function returns early, but gcc does not have\n> enough information to figure out that this is an error return.\n> ...\n> Anyway, maybe this can provide some amusement.\n\nIt actually provides only puzzlement. That is obviously a buggy compiler\nwhose warning cannot be trusted.\n\nI am not suggesting to work this around on the caller side, but I wonder\nwhat effect the usual workaround to tell the compiler that we know it\ndoesn't understand the validity of this variable by saying:\n\n\tunsigned long expire = expire;\n\nhave on this?\n\nWe obviously do not want to work the compiler bug around in the callee\nside by assigning to the variable when we didn't parse anything, but the\nchange to inline config_error_nonbool() to force this particular version\nof the compiler to see the callchain through is _too_ subtle for my taste.\nThe call-chain horizon the next version of the compiler may stop looking\nmay be different, potentially shifting the compiler bug around.\n\nI see two solutions (1) do not use -Wunitialized if the compiler is known\nto be broken, or (2) initialize the variable to 0 (not the fake \"x = x\")\non the caller side at the place of the definition (yes, we would waste one\nreal assignment) if we really need to work this around, or (3) take this\npatch.\n\nWe could obviously do (2) or (3), but the thing is, I don't think we can\nhave much confidence on -Wuninitialized warnings from this compiler once\nwe go down that route. Is it _guaranteed_ that the compiler bug _always_\nerr on the false-positive side?\n\nIOW, I'd very much prefer (1) for this particular case and if somebody\nreally cares (2).\n\nWell, it indeed turned out to be amusing, at least for some definition of\nthe word ;-). I was starting to feel somewhat depressed watching news\nprograms from Japan on ustream.\n"},{"id":"163424","messageId":"7vzkov8w0h.fsf@alter.siamese.dyndns.org","threadId":"26743","inReplyTo":"20110316034232.GA26027@elie","subject":"Re: [PATCH nd/struct-pathspec] declare 1-bit bitfields to be unsigned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T05:38:54Z","receivedAt":"2011-03-16T05:38:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> As \"gcc -pedantic\" notices, a two's complement 1-bit signed integer\n> cannot represent the value '1'.\n\nThanks; this is one of the things I try to be reasonably careful about\nwhen I eyeball patches, but apparently I didn't spot it.\n\nPerhaps I should start compiling with -Woverflow.\n"},{"id":"163425","messageId":"20110316062831.GA5252@elie","threadId":"26743","inReplyTo":"7vfwqnabbi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T06:28:48Z","receivedAt":"2011-03-16T06:28:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> We could obviously do (2) or (3), but the thing is, I don't think we can\n> have much confidence on -Wuninitialized warnings from this compiler once\n> we go down that route. Is it _guaranteed_ that the compiler bug _always_\n> err on the false-positive side?\n>\n> IOW, I'd very much prefer (1) for this particular case and if somebody\n> really cares (2).\n\n*nod*. The tricky heuristics here are that (A) a call to the function\n\n extern void foo(int *var);\n\nis assumed to initialize *var (to support idioms like strbuf_init), while\n(B) no assumption is made about the return value of a call to the function\n\n extern int bar(void);\n\nThe assumption (B) is the scourge of -Wuninitialized users. It is\nalways going produce a lot of false positives --- the compiler simply\ndoesn't have enough information to completely analyze the flow through\na function. (Even if it did have enough information, completely\nsolving the problem is Turing-complete.)\n\nProblems from (A) are more rare. -Wuninitialized will produce _some_\nfalse negatives, but problems it misses would typically be due to\nfailure to check for errors and return early, which is not what\n-Wuninitialized is about.\n\nI don't mind the warning, especially since it only appears with -O3.\nWhat makes this interesting to me is that it is a reminder that the\ncompiler has very little information about the flow of control. A\nsimple\n\n #define error(...) (report_error(__VA_ARGS__), -1)\n\ncould open the door to some nice micro-optimizations.\n"},{"id":"163426","messageId":"20110316065256.GA5988@elie","threadId":"26743","inReplyTo":"20110316024959.GA24932@elie","subject":"[PATCH 0/8] more warnings and cleanups","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T06:53:15Z","receivedAt":"2011-03-16T06:53:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Here are some more patches in the same vein.\n\nPatches 1-4 fix style problems noticed with make CC=\"c89 -pedantic\"\n(mostly concerning the -pedantic rather than the c89).\n\nPatch 5 updates fast-import.c (and everyone else) to put braces on the\nsame line as \"struct foo\" --- the waste of space had bothered me every\ntime I hacked on fast-import.c so now is my chance. :)\n\nPatches 6-8 factor out some new functions to un-indent code a little.\nThe indented code was discovered in changing\n\n\tstruct pretty_print_ctx ctx = {0};\n\ninto something more verbose to appease gcc -Wmissing-field-initializers,\nwhich probably wasn't a good idea after all. :) But the by-product is\nnice.\n\nJonathan Nieder (8):\n enums: omit trailing comma for portability\n compat: make gcc bswap an inline function\n svn-fe: do not use \"return\" for tail call returning void\n vcs-svn: remove spurious semicolons\n standardize brace placement in struct definitions\n branch: split off function that writes tracking info and commit\n subject\n cherry: split off function to print output lines\n diff --submodule: split into bite-sized pieces\n\n builtin/add.c | 3 +-\n builtin/blame.c | 3 +-\n builtin/branch.c | 48 +++++++++++++----------\n builtin/grep.c | 3 +-\n builtin/index-pack.c | 6 +--\n builtin/log.c | 34 +++++++++--------\n cache.h | 2 +-\n commit.h | 3 +-\n compat/bswap.h | 18 +++++----\n config.c | 3 +-\n convert.c | 2 +-\n diff.c | 6 +--\n fast-import.c | 42 +++++++-------------\n http-push.c | 15 ++-----\n http-walker.c | 6 +--\n http.h | 15 ++-----\n merge-recursive.c | 12 ++----\n pack-check.c | 3 +-\n string-list.h | 3 +-\n submodule.c | 103 +++++++++++++++++++++++++++++--------------------\n transport-helper.c | 3 +-\n vcs-svn/repo_tree.c | 2 +-\n vcs-svn/string_pool.c | 2 +-\n vcs-svn/svndump.c | 3 +-\n 24 files changed, 164 insertions(+), 176 deletions(-)\n"},{"id":"163427","messageId":"20110316065910.GB5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 1/8] enums: omit trailing comma for portability","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T06:59:10Z","receivedAt":"2011-03-16T06:59:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Since v1.7.2-rc0~23^2~2 (Add per-repository eol normalization,\n2010-05-19), building with gcc -std=gnu89 -pedantic produces warnings\nlike the following:\n\n convert.c:21:11: warning: comma at end of enumerator list [-pedantic]\n\ngcc is right to complain --- these commas are not permitted in C89.\nIn the spirit of v1.7.2-rc0~32^2~16 (2010-05-14), remove them.\n\nSigned-off-by: Jonathan Nieder \n---\nAt first, I was worried that the fixes from the series v1.7.2-rc0~32\nhad been in vain, but in reality it seems that there are only a few\nnew C89-compatibility tweaks to make like this one.\n\n cache.h | 2 +-\n convert.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c4ef999..80f1236 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -585,7 +585,7 @@ extern enum safe_crlf safe_crlf;\n enum auto_crlf {\n \tAUTO_CRLF_FALSE = 0,\n \tAUTO_CRLF_TRUE = 1,\n-\tAUTO_CRLF_INPUT = -1,\n+\tAUTO_CRLF_INPUT = -1\n };\n \n extern enum auto_crlf auto_crlf;\ndiff --git a/convert.c b/convert.c\nindex d5aebed..7eb51b1 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -18,7 +18,7 @@ enum action {\n \tCRLF_TEXT,\n \tCRLF_INPUT,\n \tCRLF_CRLF,\n-\tCRLF_AUTO,\n+\tCRLF_AUTO\n };\n \n struct text_stat {\n-- \n1.7.4.1\n"},{"id":"163428","messageId":"20110316070049.GC5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 2/8] compat: make gcc bswap an inline function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:00:49Z","receivedAt":"2011-03-16T07:00:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Without this change, gcc -pedantic warns:\n\n cache.h: In function 'ce_to_dtype':\n cache.h:270:21: warning: ISO C forbids braced-groups within expressions [-pedantic]\n\nAn inline function is more readable anyway.\n\nSigned-off-by: Jonathan Nieder \n---\n compat/bswap.h | 18 ++++++++++--------\n 1 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 54756db..5061214 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -21,14 +21,16 @@ static inline uint32_t default_swab32(uint32_t val)\n \n #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\n \n-#define bswap32(x) ({ \\\n-\tuint32_t __res; \\\n-\tif (__builtin_constant_p(x)) { \\\n-\t\t__res = default_swab32(x); \\\n-\t} else { \\\n-\t\t__asm__(\"bswap %0\" : \"=r\" (__res) : \"0\" ((uint32_t)(x))); \\\n-\t} \\\n-\t__res; })\n+#define bswap32 git_bswap32\n+static inline uint32_t git_bswap32(uint32_t x)\n+{\n+\tuint32_t result;\n+\tif (__builtin_constant_p(x))\n+\t\tresult = default_swab32(x);\n+\telse\n+\t\t__asm__(\"bswap %0\" : \"=r\" (result) : \"0\" (x));\n+\treturn result;\n+}\n \n #elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))\n \n-- \n1.7.4.1\n"},{"id":"163429","messageId":"20110316070150.GD5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 3/8] svn-fe: do not use \"return\" for tail call returning void","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:01:50Z","receivedAt":"2011-03-16T07:01:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.7.4-rc0~184 (2010-10-04) reminds us, a void function shouldn't\ntry to return something.\n\nNoticed with gcc -pedantic:\n\n vcs-svn/svndump.c: In function 'handle_node':\n vcs-svn/svndump.c:213:3: warning: ISO C forbids 'return' with expression,\n in function returning void [-pedantic]\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex ee7c0bb..3a0a75e 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -210,7 +210,8 @@ static void handle_node(void)\n \t\tif (mark || have_props || node_ctx.srcRev)\n \t\t\tdie(\"invalid dump: deletion node has \"\n \t\t\t\t\"copyfrom info, text, or properties\");\n-\t\treturn repo_delete(node_ctx.dst);\n+\t\trepo_delete(node_ctx.dst);\n+\t\treturn;\n \t}\n \tif (node_ctx.action == NODEACT_REPLACE) {\n \t\trepo_delete(node_ctx.dst);\n-- \n1.7.4.1\n"},{"id":"163430","messageId":"20110316070242.GE5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 4/8] vcs-svn: remove spurious semicolons","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:02:42Z","receivedAt":"2011-03-16T07:02:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"trp_gen is not a statement or function call, so it should not be\nfollowed with a semicolon. Noticed by gcc -pedantic.\n\n vcs-svn/repo_tree.c:41:81: warning: ISO C does not allow extra ';'\n outside of a function [-pedantic]\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/repo_tree.c | 2 +-\n vcs-svn/string_pool.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex 491f013..207ffc3 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -38,7 +38,7 @@ static uint32_t mark;\n static int repo_dirent_name_cmp(const void *a, const void *b);\n \n /* Treap for directory entries */\n-trp_gen(static, dent_, struct repo_dirent, children, dent, repo_dirent_name_cmp);\n+trp_gen(static, dent_, struct repo_dirent, children, dent, repo_dirent_name_cmp)\n \n uint32_t next_blob_mark(void)\n {\ndiff --git a/vcs-svn/string_pool.c b/vcs-svn/string_pool.c\nindex f5b1da8..8af8d54 100644\n--- a/vcs-svn/string_pool.c\n+++ b/vcs-svn/string_pool.c\n@@ -30,7 +30,7 @@ static int node_cmp(struct node *a, struct node *b)\n }\n \n /* Build a Treap from the node structure (a trp_node w/ offset) */\n-trp_gen(static, tree_, struct node, children, node, node_cmp);\n+trp_gen(static, tree_, struct node, children, node, node_cmp)\n \n const char *pool_fetch(uint32_t entry)\n {\n-- \n1.7.4.1\n"},{"id":"163431","messageId":"20110316070834.GF5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 5/8] standardize brace placement in struct definitions","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:08:34Z","receivedAt":"2011-03-16T07:08:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In a struct definitions, unlike functions, the prevailing style is for\nthe opening brace to go on the same line as the struct name, like so:\n\n struct foo {\n\tint bar;\n\tchar *baz;\n };\n\nIndeed, grepping for 'struct [a-z_]* {$' yields about 5 times as many\nmatches as 'struct [a-z_]*$'.\n\nLinus sayeth:\n\n Heretic people all over the world have claimed that this inconsistency\n is ... well ... inconsistent, but all right-thinking people know that\n (a) K&R are _right_ and (b) K&R are right.\n\nSigned-off-by: Jonathan Nieder \n---\nThe real reason: when I read through fast-import I end up wishing I\ncould fit more struct definitions in my 31-line terminal.\n\n builtin/add.c | 3 +--\n builtin/blame.c | 3 +--\n builtin/grep.c | 3 +--\n builtin/index-pack.c | 6 ++----\n commit.h | 3 +--\n config.c | 3 +--\n diff.c | 6 ++----\n fast-import.c | 42 ++++++++++++++----------------------------\n fetch-pack.h | 3 +--\n generate-cmdlist.sh | 3 +--\n http-push.c | 15 +++++----------\n http-walker.c | 6 ++----\n http.h | 15 +++++----------\n merge-recursive.c | 12 ++++--------\n pack-check.c | 3 +--\n string-list.h | 3 +--\n transport-helper.c | 3 +--\n 17 files changed, 44 insertions(+), 88 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f7a17e4..e127d5a 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -21,8 +21,7 @@ static const char * const builtin_add_usage[] = {\n static int patch_interactive, add_interactive, edit_interactive;\n static int take_worktree_changes;\n \n-struct update_callback_data\n-{\n+struct update_callback_data {\n \tint flags;\n \tint add_errors;\n };\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex aa30ec5..f6b03f7 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1312,8 +1312,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)\n /*\n * Information on commits, used for output.\n */\n-struct commit_info\n-{\n+struct commit_info {\n \tconst char *author;\n \tconst char *author_mail;\n \tunsigned long author_time;\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 5afee2f..eaf8560 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -40,8 +40,7 @@ enum work_type {WORK_SHA1, WORK_FILE};\n * threads. The producer adds struct work_items to 'todo' and the\n * consumers pick work items from the same array.\n */\n-struct work_item\n-{\n+struct work_item {\n \tenum work_type type;\n \tchar *name;\n \ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 8dc5c0b..c7e600d 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -13,8 +13,7 @@\n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o ] [ --keep | --keep= ] [--strict] ( | --stdin [--fix-thin] [])\";\n \n-struct object_entry\n-{\n+struct object_entry {\n \tstruct pack_idx_entry idx;\n \tunsigned long size;\n \tunsigned int hdr_size;\n@@ -44,8 +43,7 @@ struct base_data {\n #define FLAG_LINK (1u<<20)\n #define FLAG_CHECKED (1u<<21)\n \n-struct delta_entry\n-{\n+struct delta_entry {\n \tunion delta_base base;\n \tint obj_no;\n };\ndiff --git a/commit.h b/commit.h\nindex 659c87c..4198513 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -68,8 +68,7 @@ enum cmit_fmt {\n \tCMIT_FMT_UNSPECIFIED\n };\n \n-struct pretty_print_context\n-{\n+struct pretty_print_context {\n \tint abbrev;\n \tconst char *subject;\n \tconst char *after_subject;\ndiff --git a/config.c b/config.c\nindex b94de8f..fa740a6 100644\n--- a/config.c\n+++ b/config.c\n@@ -20,8 +20,7 @@ static int zlib_compression_seen;\n \n const char *config_exclusive_filename = NULL;\n \n-struct config_item\n-{\n+struct config_item {\n \tstruct config_item *next;\n \tchar *name;\n \tchar *value;\ndiff --git a/diff.c b/diff.c\nindex 6640857..3fd9e0c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -615,16 +615,14 @@ static void diff_words_append(char *line, unsigned long len,\n \tbuffer->text.ptr[buffer->text.size] = '\\0';\n }\n \n-struct diff_words_style_elem\n-{\n+struct diff_words_style_elem {\n \tconst char *prefix;\n \tconst char *suffix;\n \tconst char *color; /* NULL; filled in by the setup code if\n \t\t\t * color is enabled */\n };\n \n-struct diff_words_style\n-{\n+struct diff_words_style {\n \tenum diff_words_type type;\n \tstruct diff_words_style_elem new, old, ctx;\n \tconst char *newline;\ndiff --git a/fast-import.c b/fast-import.c\nindex e24ee2c..d9f9a3f 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -170,8 +170,7 @@ Format of STDIN stream:\n #define DEPTH_BITS 13\n #define MAX_DEPTH ((1<\n---\n builtin/branch.c | 48 +++++++++++++++++++++++++++---------------------\n 1 files changed, 27 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex fe8f2fc..b9ba011 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -390,6 +390,30 @@ static int matches_merge_filter(struct commit *commit)\n \treturn (is_merged == (merge_filter == SHOW_MERGED));\n }\n \n+static void add_verbose_info(struct strbuf *out, struct ref_item *item,\n+\t\t\t int verbose, int abbrev)\n+{\n+\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n+\tconst char *sub = \" **** invalid ref ****\";\n+\tstruct commit *commit = item->commit;\n+\n+\tif (commit && !parse_commit(commit)) {\n+\t\tstruct pretty_print_context ctx = {0};\n+\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n+\t\t\t\t &subject, &ctx);\n+\t\tsub = subject.buf;\n+\t}\n+\n+\tif (item->kind == REF_LOCAL_BRANCH)\n+\t\tfill_tracking_info(&stat, item->name, verbose > 1);\n+\n+\tstrbuf_addf(out, \" %s %s%s\",\n+\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n+\t\tstat.buf, sub);\n+\tstrbuf_release(&stat);\n+\tstrbuf_release(&subject);\n+}\n+\n static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\t\t int abbrev, int current, char *prefix)\n {\n@@ -430,27 +454,9 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \n \tif (item->dest)\n \t\tstrbuf_addf(&out, \" -> %s\", item->dest);\n-\telse if (verbose) {\n-\t\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n-\t\tconst char *sub = \" **** invalid ref ****\";\n-\n-\t\tcommit = item->commit;\n-\t\tif (commit && !parse_commit(commit)) {\n-\t\t\tstruct pretty_print_context ctx = {0};\n-\t\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n-\t\t\t\t\t &subject, &ctx);\n-\t\t\tsub = subject.buf;\n-\t\t}\n-\n-\t\tif (item->kind == REF_LOCAL_BRANCH)\n-\t\t\tfill_tracking_info(&stat, item->name, verbose > 1);\n-\n-\t\tstrbuf_addf(&out, \" %s %s%s\",\n-\t\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n-\t\t\tstat.buf, sub);\n-\t\tstrbuf_release(&stat);\n-\t\tstrbuf_release(&subject);\n-\t}\n+\telse if (verbose)\n+\t\t/* \" f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h\" */\n+\t\tadd_verbose_info(&out, item, verbose, abbrev);\n \tprintf(\"%s\\n\", out.buf);\n \tstrbuf_release(&name);\n \tstrbuf_release(&out);\n-- \n1.7.4.1\n"},{"id":"163433","messageId":"20110316071248.GH5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 7/8] cherry: split off function to print output lines","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:12:48Z","receivedAt":"2011-03-16T07:12:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Readers uninterested in the details of \"git cherry\"'s output format\ncan see\n\n\tprint_commit('-', commit, verbose, abbrev);\n\nand ignore the details.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/log.c | 34 ++++++++++++++++++----------------\n 1 files changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f5ed690..99e33b3 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1352,6 +1352,23 @@ static const char * const cherry_usage[] = {\n \tNULL\n };\n \n+static void print_commit(char sign, struct commit *commit, int verbose,\n+\t\t\t int abbrev)\n+{\n+\tif (!verbose) {\n+\t\tprintf(\"%c %s\\n\", sign,\n+\t\t find_unique_abbrev(commit->object.sha1, abbrev));\n+\t} else {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct pretty_print_context ctx = {0};\n+\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit, &buf, &ctx);\n+\t\tprintf(\"%c %s %s\\n\", sign,\n+\t\t find_unique_abbrev(commit->object.sha1, abbrev),\n+\t\t buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n+}\n+\n int cmd_cherry(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -1436,22 +1453,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \t\tcommit = list->item;\n \t\tif (has_commit_patch_id(commit, &ids))\n \t\t\tsign = '-';\n-\n-\t\tif (verbose) {\n-\t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tstruct pretty_print_context ctx = {0};\n-\t\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n-\t\t\t\t\t &buf, &ctx);\n-\t\t\tprintf(\"%c %s %s\\n\", sign,\n-\t\t\t find_unique_abbrev(commit->object.sha1, abbrev),\n-\t\t\t buf.buf);\n-\t\t\tstrbuf_release(&buf);\n-\t\t}\n-\t\telse {\n-\t\t\tprintf(\"%c %s\\n\", sign,\n-\t\t\t find_unique_abbrev(commit->object.sha1, abbrev));\n-\t\t}\n-\n+\t\tprint_commit(sign, commit, verbose, abbrev);\n \t\tlist = list->next;\n \t}\n \n-- \n1.7.4.1\n"},{"id":"163434","messageId":"20110316071411.GI5988@elie","threadId":"26743","inReplyTo":"20110316065256.GA5988@elie","subject":"[PATCH 8/8] diff --submodule: split into bite-sized pieces","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T07:14:11Z","receivedAt":"2011-03-16T07:14:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Introduce two functions:\n\n - prepare_submodule_summary prepares the revision walker\n to list changes in a submodule. That is, it:\n\n * finds merge bases between the commits pointed to this\n path from before (\"left\") and after (\"right\") the change;\n * checks whether this is a fast-forward or fast-backward;\n * prepares a revision walk to list commits in the symmetric\n difference between the commits at each endpoint.\n\n It returns nonzero on error.\n\n - print_submodule_summary runs the revision walk and saves\n the result to a strbuf in --left-right format.\n\nThe goal is just readability. No functional change intended.\n\nSigned-off-by: Jonathan Nieder \n---\n submodule.c | 103 +++++++++++++++++++++++++++++++++++------------------------\n 1 files changed, 61 insertions(+), 42 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 6f1c107..e9f2b19 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -152,17 +152,69 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\tdie(\"bad --ignore-submodules argument: %s\", arg);\n }\n \n+static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n+\t\tstruct commit *left, struct commit *right,\n+\t\tint *fast_forward, int *fast_backward)\n+{\n+\tstruct commit_list *merge_bases, *list;\n+\n+\tinit_revisions(rev, NULL);\n+\tsetup_revisions(0, NULL, rev, NULL);\n+\trev->left_right = 1;\n+\trev->first_parent_only = 1;\n+\tleft->object.flags |= SYMMETRIC_LEFT;\n+\tadd_pending_object(rev, &left->object, path);\n+\tadd_pending_object(rev, &right->object, path);\n+\tmerge_bases = get_merge_bases(left, right, 1);\n+\tif (merge_bases) {\n+\t\tif (merge_bases->item == left)\n+\t\t\t*fast_forward = 1;\n+\t\telse if (merge_bases->item == right)\n+\t\t\t*fast_backward = 1;\n+\t}\n+\tfor (list = merge_bases; list; list = list->next) {\n+\t\tlist->item->object.flags |= UNINTERESTING;\n+\t\tadd_pending_object(rev, &list->item->object,\n+\t\t\tsha1_to_hex(list->item->object.sha1));\n+\t}\n+\treturn prepare_revision_walk(rev);\n+}\n+\n+static void print_submodule_summary(struct rev_info *rev, FILE *f,\n+\t\tconst char *del, const char *add, const char *reset)\n+{\n+\tstatic const char format[] = \" %m %s\";\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct commit *commit;\n+\n+\twhile ((commit = get_revision(rev))) {\n+\t\tstruct pretty_print_context ctx = {0};\n+\t\tctx.date_mode = rev->date_mode;\n+\t\tstrbuf_setlen(&sb, 0);\n+\t\tif (commit->object.flags & SYMMETRIC_LEFT) {\n+\t\t\tif (del)\n+\t\t\t\tstrbuf_addstr(&sb, del);\n+\t\t}\n+\t\telse if (add)\n+\t\t\tstrbuf_addstr(&sb, add);\n+\t\tformat_commit_message(commit, format, &sb, &ctx);\n+\t\tif (reset)\n+\t\t\tstrbuf_addstr(&sb, reset);\n+\t\tstrbuf_addch(&sb, '\\n');\n+\t\tfprintf(f, \"%s\", sb.buf);\n+\t}\n+\tstrbuf_release(&sb);\n+}\n+\n void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit, *left = left, *right = right;\n-\tstruct commit_list *merge_bases, *list;\n+\tstruct commit *left = left, *right = right;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n-\tstatic const char *format = \" %m %s\";\n \tint fast_forward = 0, fast_backward = 0;\n \n \tif (is_null_sha1(two))\n@@ -175,29 +227,10 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\t !(right = lookup_commit_reference(two)))\n \t\tmessage = \"(commits not present)\";\n \n-\tif (!message) {\n-\t\tinit_revisions(&rev, NULL);\n-\t\tsetup_revisions(0, NULL, &rev, NULL);\n-\t\trev.left_right = 1;\n-\t\trev.first_parent_only = 1;\n-\t\tleft->object.flags |= SYMMETRIC_LEFT;\n-\t\tadd_pending_object(&rev, &left->object, path);\n-\t\tadd_pending_object(&rev, &right->object, path);\n-\t\tmerge_bases = get_merge_bases(left, right, 1);\n-\t\tif (merge_bases) {\n-\t\t\tif (merge_bases->item == left)\n-\t\t\t\tfast_forward = 1;\n-\t\t\telse if (merge_bases->item == right)\n-\t\t\t\tfast_backward = 1;\n-\t\t}\n-\t\tfor (list = merge_bases; list; list = list->next) {\n-\t\t\tlist->item->object.flags |= UNINTERESTING;\n-\t\t\tadd_pending_object(&rev, &list->item->object,\n-\t\t\t\tsha1_to_hex(list->item->object.sha1));\n-\t\t}\n-\t\tif (prepare_revision_walk(&rev))\n-\t\t\tmessage = \"(revision walker failed)\";\n-\t}\n+\tif (!message &&\n+\t prepare_submodule_summary(&rev, path, left, right,\n+\t\t\t\t\t&fast_forward, &fast_backward))\n+\t\tmessage = \"(revision walker failed)\";\n \n \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n \t\tfprintf(f, \"Submodule %s contains untracked content\\n\", path);\n@@ -221,25 +254,11 @@ void show_submodule_summary(FILE *f, const char *path,\n \tfwrite(sb.buf, sb.len, 1, f);\n \n \tif (!message) {\n-\t\twhile ((commit = get_revision(&rev))) {\n-\t\t\tstruct pretty_print_context ctx = {0};\n-\t\t\tctx.date_mode = rev.date_mode;\n-\t\t\tstrbuf_setlen(&sb, 0);\n-\t\t\tif (commit->object.flags & SYMMETRIC_LEFT) {\n-\t\t\t\tif (del)\n-\t\t\t\t\tstrbuf_addstr(&sb, del);\n-\t\t\t}\n-\t\t\telse if (add)\n-\t\t\t\tstrbuf_addstr(&sb, add);\n-\t\t\tformat_commit_message(commit, format, &sb, &ctx);\n-\t\t\tif (reset)\n-\t\t\t\tstrbuf_addstr(&sb, reset);\n-\t\t\tstrbuf_addch(&sb, '\\n');\n-\t\t\tfprintf(f, \"%s\", sb.buf);\n-\t\t}\n+\t\tprint_submodule_summary(&rev, f, del, add, reset);\n \t\tclear_commit_marks(left, ~0);\n \t\tclear_commit_marks(right, ~0);\n \t}\n+\n \tstrbuf_release(&sb);\n }\n \n-- \n1.7.4.1\n"},{"id":"163439","messageId":"4D807E66.40504@viscovery.net","threadId":"26743","inReplyTo":"7vfwqnabbi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-16T09:09:58Z","receivedAt":"2011-03-16T09:09:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/16/2011 6:22, schrieb Junio C Hamano:\n> I am not suggesting to work this around on the caller side, but I wonder\n> what effect the usual workaround to tell the compiler that we know it\n> doesn't understand the validity of this variable by saying:\n> \n> \tunsigned long expire = expire;\n> \n> have on this?\n\nWell, perhaps the compiler followed the C standard more closely than it is\ngood? By the law, the assignment above provokes undefined behavior, and\nthe compiler is allowed to do anything.\n\nI think it's time that you accept patches that turn this into\n\n\tunsigned long expire = 0;\n\n-- Hannes\n"},{"id":"163441","messageId":"4D808126.8070706@viscovery.net","threadId":"26743","inReplyTo":"20110316070049.GC5988@elie","subject":"Re: [PATCH 2/8] compat: make gcc bswap an inline function","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-16T09:21:42Z","receivedAt":"2011-03-16T09:21:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/16/2011 8:00, schrieb Jonathan Nieder:\n> +static inline uint32_t git_bswap32(uint32_t x)\n> +{\n> +\tuint32_t result;\n> +\tif (__builtin_constant_p(x))\n\nCan this predicate ever be true? Isn't it false even if the function is\ninlined?\n\n> +\t\tresult = default_swab32(x);\n> +\telse\n> +\t\t__asm__(\"bswap %0\" : \"=r\" (result) : \"0\" (x));\n> +\treturn result;\n> +}\n\n-- Hannes\n"},{"id":"163445","messageId":"20110316093108.GB7886@elie","threadId":"26743","inReplyTo":"4D808126.8070706@viscovery.net","subject":"Re: [PATCH 2/8] compat: make gcc bswap an inline function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T09:31:08Z","receivedAt":"2011-03-16T09:31:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n> Am 3/16/2011 8:00, schrieb Jonathan Nieder:\n\n>> +static inline uint32_t git_bswap32(uint32_t x)\n>> +{\n>> +\tuint32_t result;\n>> +\tif (__builtin_constant_p(x))\n>\n> Can this predicate ever be true? Isn't it false even if the function is\n> inlined?\n\nIt's true if x is a constant.\n\n$ cat test.c\n#if 0\ngcc -Wall -W -O -o tryit \"$0\"\nexec ./tryit\n#else\n#include \n#include \n\n#ifdef __GNUC__\nstatic inline int constant(uint32_t x)\n{\n\treturn __builtin_constant_p(x);\n}\n#else\nstatic inline int constant(uint32_t x) { return 0; }\n#endif\n\nint main(void)\n{\n\tprintf(\"%d\\n\", constant(3 + 5));\n\treturn 0;\n}\n#endif\n$ ./test.c\n1\n"},{"id":"163446","messageId":"20110316094639.GA8180@elie","threadId":"26743","inReplyTo":"4D807E66.40504@viscovery.net","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T09:47:03Z","receivedAt":"2011-03-16T09:47:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> \tunsigned long expire = 0;\n\nThe main downside is that that prevents valgrind from discovering when\nthe variable really is used uninitialized. But I do agree that\n\n\tunsigned long expire = expire;\n\nis ugly, which is part of why I did not use the latter workaround in\nthis patch.\n\nSince the makefile already controls what options are passed to msvc,\nis there some simple way to suppress the warning from \"expire =\nexpire\"? If not, I would find it tempting to make this and similar\nexamples look like \"unsigned long expire;\", treat the warning as\nevidence that either\n\n - the code is too complicated or does not give sufficient hints\n to the compiler about control flow, or\n - the compiler has a bug\n\nand use -Wno-uninitialized for -Werror builds.\n"},{"id":"163448","messageId":"4D8088C1.5050901@viscovery.net","threadId":"26743","inReplyTo":"20110316094639.GA8180@elie","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-16T09:54:09Z","receivedAt":"2011-03-16T09:54:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/16/2011 10:47, schrieb Jonathan Nieder:\n> Since the makefile already controls what options are passed to msvc,\n> is there some simple way to suppress the warning from \"expire =\n> expire\"?\n\nI don't think so. This is not a special warning, but just\n\n\twarning C4700: uninitialized local variable 'expire' used\n\nThat is, if you disable it, you also disable it for locations where the\nwarning would be justified. That's not something that I would like to do.\n\n-- Hannes\n"},{"id":"163454","messageId":"20110316105709.GC8277@elie","threadId":"26743","inReplyTo":"4D8088C1.5050901@viscovery.net","subject":"Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T10:57:09Z","receivedAt":"2011-03-16T10:57:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> I don't think so. This is not a special warning, but just\n>\n> \twarning C4700: uninitialized local variable 'expire' used\n>\n> That is, if you disable it, you also disable it for locations where the\n> warning would be justified. That's not something that I would like to do.\n\nRight. Below is a patch to play with which gets rid of the\n'expire = expire' style suppressions.\n\nIt was produced with\n\n\tspatch -sp_file self-assignment.cocci $(git ls-files -- '*.c' '*.h')\n\nwhere self-assignment.cocci is\n\n\t@@\n\tidentifier x;\n\ttype T;\n\t@@\n\n\t- T x = x;\n\t+ T x;\n\nbut required some pre- and post-processing to work around coccinelle\nbugs. :/\n---\n builtin/cat-file.c | 2 +-\n builtin/fast-export.c | 2 +-\n builtin/rev-list.c | 2 +-\n contrib/examples/builtin-fetch--tool.c | 4 ++--\n fast-import.c | 8 ++++----\n match-trees.c | 12 ++++++------\n merge-recursive.c | 2 +-\n run-command.c | 2 +-\n submodule.c | 2 +-\n transport.c | 2 +-\n wt-status.c | 2 +-\n 11 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 94632db..31cb172 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -168,7 +168,7 @@ static int batch_one_object(const char *obj_name, int print_contents)\n \tunsigned char sha1[20];\n \tenum object_type type = 0;\n \tunsigned long size;\n-\tvoid *contents = contents;\n+\tvoid *contents;\n \n \tif (!obj_name)\n \t return 1;\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex daf1945..408156e 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -463,7 +463,7 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \tfor (i = 0; i < pending->nr; i++) {\n \t\tstruct object_array_entry *e = pending->objects + i;\n \t\tunsigned char sha1[20];\n-\t\tstruct commit *commit = commit;\n+\t\tstruct commit *commit;\n \t\tchar *full_name;\n \n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ba27d39..96d10b0 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -397,7 +397,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n \n \tif (bisect_list) {\n-\t\tint reaches = reaches, all = all;\n+\t\tint reaches, all;\n \n \t\trevs.commits = find_bisection(revs.commits, &reaches, &all,\n \t\t\t\t\t bisect_find_all);\ndiff --git a/contrib/examples/builtin-fetch--tool.c b/contrib/examples/builtin-fetch--tool.c\nindex 3140e40..c5bd034 100644\n--- a/contrib/examples/builtin-fetch--tool.c\n+++ b/contrib/examples/builtin-fetch--tool.c\n@@ -416,14 +416,14 @@ static int expand_refs_wildcard(const char *ls_remote_result, int numrefs,\n static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_result)\n {\n \tint err = 0;\n-\tint lrr_count = lrr_count, i, pass;\n+\tint lrr_count, i, pass;\n \tconst char *cp;\n \tstruct lrr {\n \t\tconst char *line;\n \t\tconst char *name;\n \t\tint namelen;\n \t\tint shown;\n-\t} *lrr_list = lrr_list;\n+\t} *lrr_list;\n \n \tfor (pass = 0; pass < 2; pass++) {\n \t\t/* pass 0 counts and allocates, pass 1 fills... */\ndiff --git a/fast-import.c b/fast-import.c\nindex e24ee2c..565d895 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2196,7 +2196,7 @@ static void file_change_m(struct branch *b)\n \tconst char *p = command_buf.buf + 2;\n \tstatic struct strbuf uq = STRBUF_INIT;\n \tconst char *endp;\n-\tstruct object_entry *oe = oe;\n+\tstruct object_entry *oe;\n \tunsigned char sha1[20];\n \tuint16_t mode, inline_data = 0;\n \n@@ -2365,7 +2365,7 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)\n {\n \tconst char *p = command_buf.buf + 2;\n \tstatic struct strbuf uq = STRBUF_INIT;\n-\tstruct object_entry *oe = oe;\n+\tstruct object_entry *oe;\n \tstruct branch *s;\n \tunsigned char sha1[20], commit_sha1[20];\n \tchar path[60];\n@@ -2525,7 +2525,7 @@ static int parse_from(struct branch *b)\n \n static struct hash_list *parse_merge(unsigned int *count)\n {\n-\tstruct hash_list *list = NULL, *n, *e = e;\n+\tstruct hash_list *list = NULL, *n, *e;\n \tconst char *from;\n \tstruct branch *s;\n \n@@ -2819,7 +2819,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n static void parse_cat_blob(void)\n {\n \tconst char *p;\n-\tstruct object_entry *oe = oe;\n+\tstruct object_entry *oe;\n \tunsigned char sha1[20];\n \n \t/* cat-blob SP LF */\ndiff --git a/match-trees.c b/match-trees.c\nindex 26f7ed1..9cfcc8b 100644\n--- a/match-trees.c\n+++ b/match-trees.c\n@@ -72,12 +72,12 @@ static int score_trees(const unsigned char *hash1, const unsigned char *hash2)\n \t\tdie(\"%s is not a tree\", sha1_to_hex(hash2));\n \tinit_tree_desc(&two, two_buf, size);\n \twhile (one.size | two.size) {\n-\t\tconst unsigned char *elem1 = elem1;\n-\t\tconst unsigned char *elem2 = elem2;\n-\t\tconst char *path1 = path1;\n-\t\tconst char *path2 = path2;\n-\t\tunsigned mode1 = mode1;\n-\t\tunsigned mode2 = mode2;\n+\t\tconst unsigned char *elem1;\n+\t\tconst unsigned char *elem2;\n+\t\tconst char *path1;\n+\t\tconst char *path2;\n+\t\tunsigned mode1;\n+\t\tunsigned mode2;\n \t\tint cmp;\n \n \t\tif (one.size)\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 2a4f739..8ed0f29 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1583,7 +1583,7 @@ int merge_recursive(struct merge_options *o,\n {\n \tstruct commit_list *iter;\n \tstruct commit *merged_common_ancestors;\n-\tstruct tree *mrtree = mrtree;\n+\tstruct tree *mrtree;\n \tint clean;\n \n \tif (show(o, 4)) {\ndiff --git a/run-command.c b/run-command.c\nindex f91e446..f4fb936 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -139,7 +139,7 @@ int start_command(struct child_process *cmd)\n {\n \tint need_in, need_out, need_err;\n \tint fdin[2], fdout[2], fderr[2];\n-\tint failed_errno = failed_errno;\n+\tint failed_errno;\n \n \t/*\n \t * In case of errors we must keep the promise to close FDs\ndiff --git a/submodule.c b/submodule.c\nindex 6f1c107..dcbb2d3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -158,7 +158,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit, *left = left, *right = right;\n+\tstruct commit *commit, *left, *right;\n \tstruct commit_list *merge_bases, *list;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\ndiff --git a/transport.c b/transport.c\nindex 0078660..718605f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -104,7 +104,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)\n \t\treturn;\n \n \tfor (;;) {\n-\t\tint cmp = cmp, len;\n+\t\tint cmp, len;\n \n \t\tif (!fgets(buffer, sizeof(buffer), f)) {\n \t\t\tfclose(f);\ndiff --git a/wt-status.c b/wt-status.c\nindex 4daa8bb..3298897 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -153,7 +153,7 @@ static void wt_status_print_change_data(struct wt_status *s,\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst char *c = color(change_type, s);\n-\tint status = status;\n+\tint status;\n \tchar *one_name;\n \tchar *two_name;\n \tconst char *one, *two;\n-- \n1.7.4.1\n"},{"id":"163459","messageId":"20110316113540.GB15739@elie","threadId":"26743","inReplyTo":"20110316105709.GC8277@elie","subject":"[RFC/PATCH 0/6] silence -Wuninitialized warnings that previously used the a = a trick","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:35:40Z","receivedAt":"2011-03-16T11:35:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Right. Below is a patch to play with which gets rid of the\n> 'expire = expire' style suppressions.\n\nAnd here are patches on top to squash the remaining \"may be used\nuninitialized\" warnings (from gcc-4.6 -O2) that they were meant to\naddress.\n\nThe change descriptions are placeholders. In an ideal world, someone\ninterested might pick these up and rebase against master\nto skip the relevant parts of the above patch altogether.\n\nJonathan Nieder (6):\n match-trees: kill off remaining -Wuninitialized warning\n run-command: initialize failed_errno to 0\n diff --submodule: suppress -Wuninitialized warning by initializing to\n NULL\n rsync transport: clarify insert_packed_refs\n wt-status: protect against invalid change_type\n fast-import: suppress -Wuninitialized warning by initializing to NULL\n\n fast-import.c | 2 +-\n match-trees.c | 7 +++++--\n run-command.c | 2 +-\n submodule.c | 2 +-\n transport.c | 35 +++++++++++++++++++----------------\n wt-status.c | 2 ++\n 6 files changed, 29 insertions(+), 21 deletions(-)\n"},{"id":"163460","messageId":"20110316113629.GC15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 1/6] match-trees: kill off remaining -Wuninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:36:29Z","receivedAt":"2011-03-16T11:36:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Initialize to an invalid value (0, in this case) so we can still\ncatch bugs as the code evolves.\n\nSigned-off-by: Jonathan Nieder \n---\n match-trees.c | 7 +++++--\n 1 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/match-trees.c b/match-trees.c\nindex 9cfcc8b..f325ff5 100644\n--- a/match-trees.c\n+++ b/match-trees.c\n@@ -6,6 +6,7 @@ static int score_missing(unsigned mode, const char *path)\n {\n \tint score;\n \n+\tassert(mode);\n \tif (S_ISDIR(mode))\n \t\tscore = -1000;\n \telse if (S_ISLNK(mode))\n@@ -19,6 +20,7 @@ static int score_differs(unsigned mode1, unsigned mode2, const char *path)\n {\n \tint score;\n \n+\tassert(mode1 && mode2);\n \tif (S_ISDIR(mode1) != S_ISDIR(mode2))\n \t\tscore = -100;\n \telse if (S_ISLNK(mode1) != S_ISLNK(mode2))\n@@ -32,6 +34,8 @@ static int score_matches(unsigned mode1, unsigned mode2, const char *path)\n {\n \tint score;\n \n+\tassert(mode1 && mode2);\n+\n \t/* Heh, we found SHA-1 collisions between different kind of objects */\n \tif (S_ISDIR(mode1) != S_ISDIR(mode2))\n \t\tscore = -100;\n@@ -76,8 +80,7 @@ static int score_trees(const unsigned char *hash1, const unsigned char *hash2)\n \t\tconst unsigned char *elem2;\n \t\tconst char *path1;\n \t\tconst char *path2;\n-\t\tunsigned mode1;\n-\t\tunsigned mode2;\n+\t\tunsigned mode1 = 0, mode2 = 0;\n \t\tint cmp;\n \n \t\tif (one.size)\n-- \n1.7.4.1\n"},{"id":"163461","messageId":"20110316113652.GD15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 2/6] run-command: initialize failed_errno to 0","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:36:52Z","receivedAt":"2011-03-16T11:36:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is much better to report \"Success\" than a seemingly random error\nmessage based on an uninitialized value.\n\nSigned-off-by: Jonathan Nieder \n---\n run-command.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex f4fb936..8bb4d62 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -139,7 +139,7 @@ int start_command(struct child_process *cmd)\n {\n \tint need_in, need_out, need_err;\n \tint fdin[2], fdout[2], fderr[2];\n-\tint failed_errno;\n+\tint failed_errno = 0;\n \n \t/*\n \t * In case of errors we must keep the promise to close FDs\n-- \n1.7.4.1\n"},{"id":"163462","messageId":"20110316113711.GE15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 3/6] diff --submodule: suppress -Wuninitialized warning by initializing to NULL","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:37:11Z","receivedAt":"2011-03-16T11:37:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Signed-off-by: Jonathan Nieder \n---\n submodule.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex dcbb2d3..a7a7203 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -158,7 +158,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit, *left, *right;\n+\tstruct commit *commit, *left = NULL, *right = NULL;\n \tstruct commit_list *merge_bases, *list;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n-- \n1.7.4.1\n"},{"id":"163463","messageId":"20110316113731.GF15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 4/6] rsync transport: clarify insert_packed_refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:37:31Z","receivedAt":"2011-03-16T11:37:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"gcc -Wuninitialized warns us that it might be confusing.\n\nSigned-off-by: Jonathan Nieder \n---\n transport.c | 35 +++++++++++++++++++----------------\n 1 files changed, 19 insertions(+), 16 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 718605f..0f06720 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -104,7 +104,8 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)\n \t\treturn;\n \n \tfor (;;) {\n-\t\tint cmp, len;\n+\t\tint cmp = 1, len;\n+\t\tstruct ref *next;\n \n \t\tif (!fgets(buffer, sizeof(buffer), f)) {\n \t\t\tfclose(f);\n@@ -118,22 +119,24 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)\n \t\t\tbuffer[--len] = '\\0';\n \t\tif (len < 41)\n \t\t\tcontinue;\n-\t\twhile ((*list)->next &&\n-\t\t\t\t(cmp = strcmp(buffer + 41,\n-\t\t\t\t (*list)->next->name)) > 0)\n-\t\t\tlist = &(*list)->next;\n-\t\tif (!(*list)->next || cmp < 0) {\n-\t\t\tstruct ref *next = alloc_ref(buffer + 41);\n-\t\t\tbuffer[40] = '\\0';\n-\t\t\tif (get_sha1_hex(buffer, next->old_sha1)) {\n-\t\t\t\twarning (\"invalid SHA-1: %s\", buffer);\n-\t\t\t\tfree(next);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tnext->next = (*list)->next;\n-\t\t\t(*list)->next = next;\n-\t\t\tlist = &(*list)->next;\n+\t\tfor (; (*list)->next; list = &(*list)->next) {\n+\t\t\tcmp = strcmp(buffer + 41, (*list)->next->name);\n+\t\t\tif (cmp <= 0)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (!cmp)\t/* already inserted. */\n+\t\t\tcontinue;\n+\n+\t\tnext = alloc_ref(buffer + 41);\n+\t\tbuffer[40] = '\\0';\n+\t\tif (get_sha1_hex(buffer, next->old_sha1)) {\n+\t\t\twarning (\"invalid SHA-1: %s\", buffer);\n+\t\t\tfree(next);\n+\t\t\tcontinue;\n \t\t}\n+\t\tnext->next = (*list)->next;\n+\t\t(*list)->next = next;\n+\t\tlist = &(*list)->next;\n \t}\n }\n \n-- \n1.7.4.1\n"},{"id":"163464","messageId":"20110316113750.GG15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 5/6] wt-status: protect against invalid change_type","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:37:50Z","receivedAt":"2011-03-16T11:37:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Noticed with gcc -Wuninitialized.\n\nSigned-off-by: Jonathan Nieder \n---\n wt-status.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 3298897..25a411b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -181,6 +181,8 @@ static void wt_status_print_change_data(struct wt_status *s,\n \t\t}\n \t\tstatus = d->worktree_status;\n \t\tbreak;\n+\tdefault:\n+\t\tdie(\"BUG: invalid change_type\");\n \t}\n \n \tone = quote_path(one_name, -1, &onebuf, s->prefix);\n-- \n1.7.4.1\n"},{"id":"163465","messageId":"20110316113842.GH15739@elie","threadId":"26743","inReplyTo":"20110316113540.GB15739@elie","subject":"[PATCH 6/6] fast-import: suppress -Wuninitialized warning by initializing to NULL","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:38:42Z","receivedAt":"2011-03-16T11:38:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Signed-off-by: Jonathan Nieder \n---\nThat's the end. :)\n\nGood night.\n\n fast-import.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 565d895..75405af 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2196,7 +2196,7 @@ static void file_change_m(struct branch *b)\n \tconst char *p = command_buf.buf + 2;\n \tstatic struct strbuf uq = STRBUF_INIT;\n \tconst char *endp;\n-\tstruct object_entry *oe;\n+\tstruct object_entry *oe = NULL;\n \tunsigned char sha1[20];\n \tuint16_t mode, inline_data = 0;\n \n-- \n1.7.4.1\n"},{"id":"163473","messageId":"AANLkTi=0ifczJw-Thr-XD7JX0YbQbvczSYrWOTB-Pru9@mail.gmail.com","threadId":"26743","inReplyTo":"20110316034232.GA26027@elie","subject":"Re: [PATCH nd/struct-pathspec] declare 1-bit bitfields to be unsigned","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-16T14:20:43Z","receivedAt":"2011-03-16T14:20:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/3/16 Jonathan Nieder :\n> As \"gcc -pedantic\" notices, a two's complement 1-bit signed integer\n> cannot represent the value '1'.\n>\n>  dir.c: In function 'init_pathspec':\n>  dir.c:1291:4: warning: overflow in implicit constant conversion [-Woverflow]\n\nThanks. I think I was aware of this but neglected because they are\nused as booleans, 1 or -1 does not matter. Good fix anyway.\n-- \nDuy\n"},{"id":"163498","messageId":"4D8104DC.2010700@web.de","threadId":"26743","inReplyTo":"20110316071411.GI5988@elie","subject":"Re: [PATCH 8/8] diff --submodule: split into bite-sized pieces","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-03-16T18:43:40Z","receivedAt":"2011-03-16T18:43:40Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.03.2011 08:14, schrieb Jonathan Nieder:\n> Introduce two functions:\n> \n> - prepare_submodule_summary prepares the revision walker\n> to list changes in a submodule. That is, it:\n> \n> * finds merge bases between the commits pointed to this\n> path from before (\"left\") and after (\"right\") the change;\n> * checks whether this is a fast-forward or fast-backward;\n> * prepares a revision walk to list commits in the symmetric\n> difference between the commits at each endpoint.\n> \n> It returns nonzero on error.\n> \n> - print_submodule_summary runs the revision walk and saves\n> the result to a strbuf in --left-right format.\n> \n> The goal is just readability. No functional change intended.\n\nAck, looks good and makes sense to me.\n\n\n> Signed-off-by: Jonathan Nieder \n> ---\n> submodule.c | 103 +++++++++++++++++++++++++++++++++++------------------------\n> 1 files changed, 61 insertions(+), 42 deletions(-)\n> \n> diff --git a/submodule.c b/submodule.c\n> index 6f1c107..e9f2b19 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -152,17 +152,69 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n> \t\tdie(\"bad --ignore-submodules argument: %s\", arg);\n> }\n> \n> +static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n> +\t\tstruct commit *left, struct commit *right,\n> +\t\tint *fast_forward, int *fast_backward)\n> +{\n> +\tstruct commit_list *merge_bases, *list;\n> +\n> +\tinit_revisions(rev, NULL);\n> +\tsetup_revisions(0, NULL, rev, NULL);\n> +\trev->left_right = 1;\n> +\trev->first_parent_only = 1;\n> +\tleft->object.flags |= SYMMETRIC_LEFT;\n> +\tadd_pending_object(rev, &left->object, path);\n> +\tadd_pending_object(rev, &right->object, path);\n> +\tmerge_bases = get_merge_bases(left, right, 1);\n> +\tif (merge_bases) {\n> +\t\tif (merge_bases->item == left)\n> +\t\t\t*fast_forward = 1;\n> +\t\telse if (merge_bases->item == right)\n> +\t\t\t*fast_backward = 1;\n> +\t}\n> +\tfor (list = merge_bases; list; list = list->next) {\n> +\t\tlist->item->object.flags |= UNINTERESTING;\n> +\t\tadd_pending_object(rev, &list->item->object,\n> +\t\t\tsha1_to_hex(list->item->object.sha1));\n> +\t}\n> +\treturn prepare_revision_walk(rev);\n> +}\n> +\n> +static void print_submodule_summary(struct rev_info *rev, FILE *f,\n> +\t\tconst char *del, const char *add, const char *reset)\n> +{\n> +\tstatic const char format[] = \" %m %s\";\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct commit *commit;\n> +\n> +\twhile ((commit = get_revision(rev))) {\n> +\t\tstruct pretty_print_context ctx = {0};\n> +\t\tctx.date_mode = rev->date_mode;\n> +\t\tstrbuf_setlen(&sb, 0);\n> +\t\tif (commit->object.flags & SYMMETRIC_LEFT) {\n> +\t\t\tif (del)\n> +\t\t\t\tstrbuf_addstr(&sb, del);\n> +\t\t}\n> +\t\telse if (add)\n> +\t\t\tstrbuf_addstr(&sb, add);\n> +\t\tformat_commit_message(commit, format, &sb, &ctx);\n> +\t\tif (reset)\n> +\t\t\tstrbuf_addstr(&sb, reset);\n> +\t\tstrbuf_addch(&sb, '\\n');\n> +\t\tfprintf(f, \"%s\", sb.buf);\n> +\t}\n> +\tstrbuf_release(&sb);\n> +}\n> +\n> void show_submodule_summary(FILE *f, const char *path,\n> \t\tunsigned char one[20], unsigned char two[20],\n> \t\tunsigned dirty_submodule,\n> \t\tconst char *del, const char *add, const char *reset)\n> {\n> \tstruct rev_info rev;\n> -\tstruct commit *commit, *left = left, *right = right;\n> -\tstruct commit_list *merge_bases, *list;\n> +\tstruct commit *left = left, *right = right;\n> \tconst char *message = NULL;\n> \tstruct strbuf sb = STRBUF_INIT;\n> -\tstatic const char *format = \" %m %s\";\n> \tint fast_forward = 0, fast_backward = 0;\n> \n> \tif (is_null_sha1(two))\n> @@ -175,29 +227,10 @@ void show_submodule_summary(FILE *f, const char *path,\n> \t\t !(right = lookup_commit_reference(two)))\n> \t\tmessage = \"(commits not present)\";\n> \n> -\tif (!message) {\n> -\t\tinit_revisions(&rev, NULL);\n> -\t\tsetup_revisions(0, NULL, &rev, NULL);\n> -\t\trev.left_right = 1;\n> -\t\trev.first_parent_only = 1;\n> -\t\tleft->object.flags |= SYMMETRIC_LEFT;\n> -\t\tadd_pending_object(&rev, &left->object, path);\n> -\t\tadd_pending_object(&rev, &right->object, path);\n> -\t\tmerge_bases = get_merge_bases(left, right, 1);\n> -\t\tif (merge_bases) {\n> -\t\t\tif (merge_bases->item == left)\n> -\t\t\t\tfast_forward = 1;\n> -\t\t\telse if (merge_bases->item == right)\n> -\t\t\t\tfast_backward = 1;\n> -\t\t}\n> -\t\tfor (list = merge_bases; list; list = list->next) {\n> -\t\t\tlist->item->object.flags |= UNINTERESTING;\n> -\t\t\tadd_pending_object(&rev, &list->item->object,\n> -\t\t\t\tsha1_to_hex(list->item->object.sha1));\n> -\t\t}\n> -\t\tif (prepare_revision_walk(&rev))\n> -\t\t\tmessage = \"(revision walker failed)\";\n> -\t}\n> +\tif (!message &&\n> +\t prepare_submodule_summary(&rev, path, left, right,\n> +\t\t\t\t\t&fast_forward, &fast_backward))\n> +\t\tmessage = \"(revision walker failed)\";\n> \n> \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n> \t\tfprintf(f, \"Submodule %s contains untracked content\\n\", path);\n> @@ -221,25 +254,11 @@ void show_submodule_summary(FILE *f, const char *path,\n> \tfwrite(sb.buf, sb.len, 1, f);\n> \n> \tif (!message) {\n> -\t\twhile ((commit = get_revision(&rev))) {\n> -\t\t\tstruct pretty_print_context ctx = {0};\n> -\t\t\tctx.date_mode = rev.date_mode;\n> -\t\t\tstrbuf_setlen(&sb, 0);\n> -\t\t\tif (commit->object.flags & SYMMETRIC_LEFT) {\n> -\t\t\t\tif (del)\n> -\t\t\t\t\tstrbuf_addstr(&sb, del);\n> -\t\t\t}\n> -\t\t\telse if (add)\n> -\t\t\t\tstrbuf_addstr(&sb, add);\n> -\t\t\tformat_commit_message(commit, format, &sb, &ctx);\n> -\t\t\tif (reset)\n> -\t\t\t\tstrbuf_addstr(&sb, reset);\n> -\t\t\tstrbuf_addch(&sb, '\\n');\n> -\t\t\tfprintf(f, \"%s\", sb.buf);\n> -\t\t}\n> +\t\tprint_submodule_summary(&rev, f, del, add, reset);\n> \t\tclear_commit_marks(left, ~0);\n> \t\tclear_commit_marks(right, ~0);\n> \t}\n> +\n> \tstrbuf_release(&sb);\n> }\n> \n"},{"id":"163500","messageId":"20110316193350.GA8179@elie","threadId":"26743","inReplyTo":"4D8104DC.2010700@web.de","subject":"Re: [PATCH 8/8] diff --submodule: split into bite-sized pieces","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T19:33:50Z","receivedAt":"2011-03-16T19:33:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jens Lehmann wrote:\n> Am 16.03.2011 08:14, schrieb Jonathan Nieder:\n\n>> Introduce two functions:\n>> \n>> - prepare_submodule_summary prepares the revision walker\n>> to list changes in a submodule. That is, it:\n>> \n>> * finds merge bases between the commits pointed to this\n>> path from before (\"left\") and after (\"right\") the change;\n\npointed to at this path before [missing \"at\", spurious \"from\"]\n\n>> * checks whether this is a fast-forward or fast-backward;\n>> * prepares a revision walk to list commits in the symmetric\n>> difference between the commits at each endpoint.\n>>\n>> It returns nonzero on error.\n>>\n>> - print_submodule_summary runs the revision walk and saves\n>> the result to a strbuf in --left-right format.\n\nruns the revision walk and prints the result in --left-right format\n[the strbuf is an implementation detail; not sure how it snuck into\nthe commit message]\n\n>> The goal is just readability. No functional change intended.\n>\n> Ack, looks good and makes sense to me.\n\nThanks, Jens. Looking back over the commit message I see I left in\nsome typos but the patch still looks good to me fwiw.\n"},{"id":"163501","messageId":"7v8vwe97f9.fsf@alter.siamese.dyndns.org","threadId":"26743","inReplyTo":"20110316093108.GB7886@elie","subject":"Re: [PATCH 2/8] compat: make gcc bswap an inline function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T19:44:42Z","receivedAt":"2011-03-16T19:44:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Johannes Sixt wrote:\n>> Am 3/16/2011 8:00, schrieb Jonathan Nieder:\n>\n>>> +static inline uint32_t git_bswap32(uint32_t x)\n>>> +{\n>>> +\tuint32_t result;\n>>> +\tif (__builtin_constant_p(x))\n>>\n>> Can this predicate ever be true? Isn't it false even if the function is\n>> inlined?\n>\n> It's true if x is a constant.\n> ... demonstration ...\n\nAnd this trivial function:\n\n int f(void) {\n return git_bswap32(0x01000000);\n }\n\ncompiles down to:\n\n .globl f\n .type f, @function\n f:\n .LFB2:\n .cfi_startproc\n movl $1, %eax\n ret\n .cfi_endproc\n\non my amd64, with -Os, -O1, or -O2 (gcc 4.4.5). Of course -O0 gives a true\ncall to git_bswap32() and leaves the definitions of two helper functions\nin the object, but that is to be expected.\n"},{"id":"163502","messageId":"7v4o72979u.fsf@alter.siamese.dyndns.org","threadId":"26743","inReplyTo":"20110316070242.GE5988@elie","subject":"Re: [PATCH 4/8] vcs-svn: remove spurious semicolons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T19:47:57Z","receivedAt":"2011-03-16T19:47:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> trp_gen is not a statement or function call, so it should not be\n> followed with a semicolon. Noticed by gcc -pedantic.\n>\n> vcs-svn/repo_tree.c:41:81: warning: ISO C does not allow extra ';'\n> outside of a function [-pedantic]\n> ...\n> diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\n> index 491f013..207ffc3 100644\n> --- a/vcs-svn/repo_tree.c\n> +++ b/vcs-svn/repo_tree.c\n> @@ -38,7 +38,7 @@ static uint32_t mark;\n> static int repo_dirent_name_cmp(const void *a, const void *b);\n> \n> /* Treap for directory entries */\n> -trp_gen(static, dent_, struct repo_dirent, children, dent, repo_dirent_name_cmp);\n> +trp_gen(static, dent_, struct repo_dirent, children, dent, repo_dirent_name_cmp)\n\nYuck. Correct but ugly.\n"},{"id":"163503","messageId":"20110316200341.GA8685@elie","threadId":"26743","inReplyTo":"7v4o72979u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/8] vcs-svn: remove spurious semicolons","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T20:03:42Z","receivedAt":"2011-03-16T20:03:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Yuck. Correct but ugly.\n\nYes, might be better for me to just hurry and apply David's patches to\nget rid of the treap. :)\n"},{"id":"163612","messageId":"7vwrjwyjof.fsf@alter.siamese.dyndns.org","threadId":"26743","inReplyTo":"20110316070834.GF5988@elie","subject":"Re: [PATCH 5/8] standardize brace placement in struct definitions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-18T07:25:20Z","receivedAt":"2011-03-18T07:25:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> In a struct definitions, unlike functions, the prevailing style is for\n> the opening brace to go on the same line as the struct name, like so:\n>\n> struct foo {\n> \tint bar;\n> \tchar *baz;\n> };\n\nThanks for killing these eyesores.\n"}]}