{"thread":{"id":"38361","subject":"[PATCH] init-db: remove unused #includes","startedAt":"2015-01-14T17:59:27Z","lastAt":"2015-01-15T23:40:04Z","messageCount":8,"participants":["Alexander Kuleshov","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"254668","messageId":"1421258367-8483-1-git-send-email-kuleshovmail@gmail.com","threadId":"38361","inReplyTo":null,"subject":"[PATCH] init-db: remove unused #includes","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2015-01-14T17:59:27Z","receivedAt":"2015-01-14T17:59:27Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"* \"cache.h\" - is unnecessary because it already included at \"builtin.h\"\n\n* \"exec_cmd.h\" - was added at a47d1813 (Allow a relative builtin template\ndirectory., 15 Nov 2007). init-db used 'git_exec_path' routine from\n\"exec_cmd.h\", but later it was removed at 2de9de5e (Move code interpreting\npath relative to exec-dir to new function system_path()., 14 Jul 2008). So\nwe no need in it anymore.\n\nSigned-off-by: Alexander Kuleshov \n---\n builtin/init-db.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 280454a..2978b36 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -3,9 +3,7 @@\n *\n * Copyright (C) Linus Torvalds, 2005\n */\n-#include \"cache.h\"\n #include \"builtin.h\"\n-#include \"exec_cmd.h\"\n #include \"parse-options.h\"\n \n #ifndef DEFAULT_GIT_TEMPLATE_DIR\n-- \n2.3.0.rc0.255.g53b80d0\n"},{"id":"254681","messageId":"xmqq61c99o25.fsf@gitster.dls.corp.google.com","threadId":"38361","inReplyTo":"1421258367-8483-1-git-send-email-kuleshovmail@gmail.com","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-14T19:09:38Z","receivedAt":"2015-01-14T19:09:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Kuleshov writes:\n\n> * \"cache.h\" - is unnecessary because it already included at \"builtin.h\"\n>\n> * \"exec_cmd.h\" - was added at a47d1813 (Allow a relative builtin template\n> directory., 15 Nov 2007). init-db used 'git_exec_path' routine from\n> \"exec_cmd.h\", but later it was removed at 2de9de5e (Move code interpreting\n> path relative to exec-dir to new function system_path()., 14 Jul 2008). So\n> we no need in it anymore.\n>\n> Signed-off-by: Alexander Kuleshov \n> ---\n\nMakes sense. Thanks.\n\n> builtin/init-db.c | 2 --\n> 1 file changed, 2 deletions(-)\n>\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index 280454a..2978b36 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -3,9 +3,7 @@\n> *\n> * Copyright (C) Linus Torvalds, 2005\n> */\n> -#include \"cache.h\"\n> #include \"builtin.h\"\n> -#include \"exec_cmd.h\"\n> #include \"parse-options.h\"\n> \n> #ifndef DEFAULT_GIT_TEMPLATE_DIR\n"},{"id":"254685","messageId":"xmqqwq4p86i6.fsf@gitster.dls.corp.google.com","threadId":"38361","inReplyTo":"xmqq61c99o25.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-14T20:14:09Z","receivedAt":"2015-01-14T20:14:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Alexander Kuleshov writes:\n>\n>> * \"cache.h\" - is unnecessary because it already included at \"builtin.h\"\n>>\n>> * \"exec_cmd.h\" - was added at a47d1813 (Allow a relative builtin template\n>> directory., 15 Nov 2007). init-db used 'git_exec_path' routine from\n>> \"exec_cmd.h\", but later it was removed at 2de9de5e (Move code interpreting\n>> path relative to exec-dir to new function system_path()., 14 Jul 2008). So\n>> we no need in it anymore.\n>>\n>> Signed-off-by: Alexander Kuleshov \n>> ---\n>\n> Makes sense. Thanks.\n\nActually, not. 2de9de5e does make it unnecessary to refer to\ngit_exec_path(), but in order to reference the newly introduced\nsystem_path(), which is declared in exec_cmd.h, the file still would\nhave needed to include the header, so the justification for the\nlatter does not make sense.\n\nAnd indeed, we still need exec_cmd.h in today's code.\n"},{"id":"254737","messageId":"CANCZXo5UY80327pvjzYkJXroH_WZdW36LhSANK0O7GGVQLF=AA@mail.gmail.com","threadId":"38361","inReplyTo":"xmqqwq4p86i6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2015-01-15T10:13:00Z","receivedAt":"2015-01-15T10:13:00Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"Hello Junio,\n\nyes right, missed system_path usage. But it's strange, code still\ncompiles successfully without exec_cmd.h.\n"},{"id":"254740","messageId":"20150115105558.GA13536@peff.net","threadId":"38361","inReplyTo":"CANCZXo5UY80327pvjzYkJXroH_WZdW36LhSANK0O7GGVQLF=AA@mail.gmail.com","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-01-15T10:55:59Z","receivedAt":"2015-01-15T10:55:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 15, 2015 at 04:13:00PM +0600, Alexander Kuleshov wrote:\n\n> yes right, missed system_path usage. But it's strange, code still\n> compiles successfully without exec_cmd.h.\n\nSort of. With your patch I get:\n\nbuiltin/init-db.c: In function ‘copy_templates’:\nbuiltin/init-db.c:127:3: warning: implicit declaration of function ‘system_path’ [-Wimplicit-function-declaration]\n template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n ^\nbuiltin/init-db.c:127:26: warning: assignment makes pointer from integer without a cast\n template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n\nThose are just warnings, and on some platforms the resulting code will\nbe fine, but it does violate the C standard to not have a valid\ndeclaration in this case (it defaults to an int return type, which is\nwrong; but simply as a matter of style we do not like implicit\ndeclarations even when they are technically correct).\n\nIf you are going to be doing refactoring, I'd recommend building with\n\"-Werror\" in your CFLAGS to catch such problems.\n\nFWIW, the full set of CFLAGS I use (which I specify in config.mak) is:\n\n CFLAGS += -Wall -Werror\n CFLAGS += -Wno-format-zero-length\n CFLAGS += -Wdeclaration-after-statement\n CFLAGS += -Wpointer-arith\n CFLAGS += -Wstrict-prototypes\n CFLAGS += -Wold-style-declaration\n\n-Peff\n"},{"id":"254774","messageId":"xmqqsifb6atr.fsf@gitster.dls.corp.google.com","threadId":"38361","inReplyTo":"20150115105558.GA13536@peff.net","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-15T20:36:00Z","receivedAt":"2015-01-15T20:36:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> FWIW, the full set of CFLAGS I use (which I specify in config.mak) is:\n>\n> CFLAGS += -Wall -Werror\n> CFLAGS += -Wno-format-zero-length\n> CFLAGS += -Wdeclaration-after-statement\n> CFLAGS += -Wpointer-arith\n> CFLAGS += -Wstrict-prototypes\n> CFLAGS += -Wold-style-declaration\n>\n> -Peff\n\nI think I have no-pointer-to-int-cast, old-style-definition and vla\nin addition to the above.\n"},{"id":"254780","messageId":"20150115223112.GB19021@peff.net","threadId":"38361","inReplyTo":"xmqqsifb6atr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-01-15T22:31:12Z","receivedAt":"2015-01-15T22:31:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 15, 2015 at 12:36:00PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > FWIW, the full set of CFLAGS I use (which I specify in config.mak) is:\n> >\n> > CFLAGS += -Wall -Werror\n> > CFLAGS += -Wno-format-zero-length\n> > CFLAGS += -Wdeclaration-after-statement\n> > CFLAGS += -Wpointer-arith\n> > CFLAGS += -Wstrict-prototypes\n> > CFLAGS += -Wold-style-declaration\n> \n> I think I have no-pointer-to-int-cast, old-style-definition and vla\n> in addition to the above.\n\nThanks, I added the latter two to my setup. But what is the purpose of\nturning off pointer-to-int warnings? It seems like those are a good\nindication of a sloppy construct (and AFAICT, we do not have any code\nwhich triggers on it).\n\n-Peff\n"},{"id":"254790","messageId":"xmqqlhl34nqj.fsf@gitster.dls.corp.google.com","threadId":"38361","inReplyTo":"20150115223112.GB19021@peff.net","subject":"Re: [PATCH] init-db: remove unused #includes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-15T23:40:04Z","receivedAt":"2015-01-15T23:40:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jan 15, 2015 at 12:36:00PM -0800, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > FWIW, the full set of CFLAGS I use (which I specify in config.mak) is:\n>> >\n>> > CFLAGS += -Wall -Werror\n>> > CFLAGS += -Wno-format-zero-length\n>> > CFLAGS += -Wdeclaration-after-statement\n>> > CFLAGS += -Wpointer-arith\n>> > CFLAGS += -Wstrict-prototypes\n>> > CFLAGS += -Wold-style-declaration\n>> \n>> I think I have no-pointer-to-int-cast, old-style-definition and vla\n>> in addition to the above.\n>\n> Thanks, I added the latter two to my setup. But what is the purpose of\n> turning off pointer-to-int warnings? It seems like those are a good\n> indication of a sloppy construct (and AFAICT, we do not have any code\n> which triggers on it).\n\nIt probably a remnant from olden days; perhaps we used to have a\ncode that stuffs a pointer value to an int field used as a hash key\nor something. As you said, there is no need for disabling that\ncheck in today's code.\n\nThanks for catching.\n"}]}