{"thread":{"id":"36559","subject":"split index: sparse warning","startedAt":"2014-04-30T23:44:58Z","lastAt":"2014-06-02T15:12:01Z","messageCount":4,"participants":["Ramsay Jones","Duy Nguyen","Nguyễn Thái Ngọc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"240392","messageId":"53618AFA.6070602@ramsay1.demon.co.uk","threadId":"36559","inReplyTo":null,"subject":"split index: sparse warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2014-04-30T23:44:58Z","receivedAt":"2014-04-30T23:44:58Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Duy,\n\nI haven't looked at your split-index series at all. However, sparse has\nnagged me to take a quick squint at the result of the series as it\nappears in the 'pu' branch.\n\nIn particular, sparse complains thus:\n\n SP sequencer.c\n sequencer.c:690:49: error: incompatible types for operation (>=)\n sequencer.c:690:49: left side has type int ( extern [toplevel] * )( ... )\n sequencer.c:690:49: right side has type int\n\n... which is fair enough; index_fd is, indeed, a function (pointer) and\nnot an int file descriptor! The offending code looks like:\n\n683 static void read_and_refresh_cache(struct replay_opts *opts)\n684 {\n685 static struct lock_file index_lock;\n686 hold_locked_index(&index_lock, 0);\n687 if (read_index_preload(&the_index, NULL) < 0)\n688 die(_(\"git %s: failed to read the index\"), action_name(opts));\n689 refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n690 if (the_index.cache_changed && index_fd >= 0) {\n691 if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))\n692 die(_(\"git %s: failed to refresh the index\"), action_name(opts));\n693 }\n694 rollback_lock_file(&index_lock);\n695 }\n\n\nIt seems that, in an earlier commit (33c297aa), index_fd was declared\nas a local int variable (hiding the global function) which was then\ninitialised by a call to hold_locked_index().\n\nI assume that the conditional should be changed to something like:\n\n690 if (the_index.cache_changed && index_lock.fd >= 0) {\n\n... but I haven't spent any time investigating this, so take this\nsuggestion which a large pinch of salt! :-P\n\nATB,\nRamsay Jones\n"},{"id":"240394","messageId":"CACsJy8Ad+hfmJSgKX6O9FTXggUjyMkpJLzf=awnV5oFACsX+vQ@mail.gmail.com","threadId":"36559","inReplyTo":"53618AFA.6070602@ramsay1.demon.co.uk","subject":"Re: split index: sparse warning","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-01T00:12:02Z","receivedAt":"2014-05-01T00:12:02Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 1, 2014 at 6:44 AM, Ramsay Jones wrote:\n> Hi Duy,\n>\n> I haven't looked at your split-index series at all. However, sparse has\n> nagged me to take a quick squint at the result of the series as it\n> appears in the 'pu' branch.\n>\n> In particular, sparse complains thus:\n>\n> SP sequencer.c\n> sequencer.c:690:49: error: incompatible types for operation (>=)\n> sequencer.c:690:49: left side has type int ( extern [toplevel] * )( ... )\n> sequencer.c:690:49: right side has type int\n>\n> ... which is fair enough; index_fd is, indeed, a function (pointer) and\n> not an int file descriptor! The offending code looks like:\n>\n> 683 static void read_and_refresh_cache(struct replay_opts *opts)\n> 684 {\n> 685 static struct lock_file index_lock;\n> 686 hold_locked_index(&index_lock, 0);\n> 687 if (read_index_preload(&the_index, NULL) < 0)\n> 688 die(_(\"git %s: failed to read the index\"), action_name(opts));\n> 689 refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n> 690 if (the_index.cache_changed && index_fd >= 0) {\n> 691 if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))\n> 692 die(_(\"git %s: failed to refresh the index\"), action_name(opts));\n> 693 }\n> 694 rollback_lock_file(&index_lock);\n> 695 }\n>\n>\n> It seems that, in an earlier commit (33c297aa), index_fd was declared\n> as a local int variable (hiding the global function) which was then\n> initialised by a call to hold_locked_index().\n\nUgh.. after removing index_fd the local variable, index_fd now becomes\nthe function from sha1_file.c. Noted and will fix it in the next\nreroll.\n--\nDuy\n"},{"id":"243083","messageId":"1401583635-1332-1-git-send-email-pclouds@gmail.com","threadId":"36559","inReplyTo":"CACsJy8Ad+hfmJSgKX6O9FTXggUjyMkpJLzf=awnV5oFACsX+vQ@mail.gmail.com","subject":"[PATCH nd/split-index] fixup! read-cache: new API write_locked_index instead of write_index/write_cache","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-01T00:47:15Z","receivedAt":"2014-06-01T00:47:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n I intended it resend the series after the comments I received, but it\n looks like Junio has picked up all comments except this one, so\n here's the fix.\n\n sequencer.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 377c877..4b709db 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -672,7 +672,7 @@ static void prepare_revs(struct replay_opts *opts)\n static void read_and_refresh_cache(struct replay_opts *opts)\n {\n \tstatic struct lock_file index_lock;\n-\thold_locked_index(&index_lock, 0);\n+\tint index_fd = hold_locked_index(&index_lock, 0);\n \tif (read_index_preload(&the_index, NULL) < 0)\n \t\tdie(_(\"git %s: failed to read the index\"), action_name(opts));\n \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"243133","messageId":"538C9441.3010804@ramsay1.demon.co.uk","threadId":"36559","inReplyTo":"1401583635-1332-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH nd/split-index] fixup! read-cache: new API write_locked_index instead of write_index/write_cache","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2014-06-02T15:12:01Z","receivedAt":"2014-06-02T15:12:01Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"On 01/06/14 01:47, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> I intended it resend the series after the comments I received, but it\n> looks like Junio has picked up all comments except this one, so\n> here's the fix.\n> \n> sequencer.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 377c877..4b709db 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -672,7 +672,7 @@ static void prepare_revs(struct replay_opts *opts)\n> static void read_and_refresh_cache(struct replay_opts *opts)\n> {\n> \tstatic struct lock_file index_lock;\n> -\thold_locked_index(&index_lock, 0);\n> +\tint index_fd = hold_locked_index(&index_lock, 0);\n> \tif (read_index_preload(&the_index, NULL) < 0)\n> \t\tdie(_(\"git %s: failed to read the index\"), action_name(opts));\n> \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n> \n\nYep, this silences sparse. I would have declared the variable to be\n(say) fd (and changed the _use_ site as well, of course!), rather\nthan once again hiding the index_fd() global function. However, this\nis perfectly fine as-is.\n\nThe actual culprit is the index_fd() global function, but renaming it\nnow is probably not worth the code churn (and I'm lazy). ;-)\n\nThanks!\n\nATB,\nRamsay Jones\n"}]}