patchinit-db: remove unused #includes

* "cache.h" - is unnecessary because it already included at "builtin.h"

* "exec_cmd.h" - was added at a47d1813 (Allow a relative builtin template
directory., 15 Nov 2007). init-db used 'git_exec_path' routine from
"exec_cmd.h", but later it was removed at 2de9de5e (Move code interpreting
path relative to exec-dir to new function system_path()., 14 Jul 2008). So
we no need in it anymore.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 builtin/init-db.c | 2 --
 1 file changed, 2 deletions(-)

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

Makes sense. Thanks.

Junio C Hamano <gitster@pobox.com> writes:

Actually, not. 2de9de5e does make it unnecessary to refer to git_exec_path(), but in order to reference the newly introduced system_path(), which is declared in exec_cmd.h, the file still would have needed to include the header, so the justification for the latter does not make sense.

And indeed, we still need exec_cmd.h in today's code.

Hello Junio,

yes right, missed system_path usage. But it's strange, code still compiles successfully without exec_cmd.h.

On Thu, Jan 15, 2015 at 04:13:00PM +0600, Alexander Kuleshov wrote:

Sort of. With your patch I get:

builtin/init-db.c: In function ‘copy_templates’:
builtin/init-db.c:127:3: warning: implicit declaration of function ‘system_path’ [-Wimplicit-function-declaration]
   template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
   ^
builtin/init-db.c:127:26: warning: assignment makes pointer from integer without a cast
   template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);

Those are just warnings, and on some platforms the resulting code will be fine, but it does violate the C standard to not have a valid declaration in this case (it defaults to an int return type, which is wrong; but simply as a matter of style we do not like implicit declarations even when they are technically correct).

If you are going to be doing refactoring, I'd recommend building with "-Werror" in your CFLAGS to catch such problems.

FWIW, the full set of CFLAGS I use (which I specify in config.mak) is:

    CFLAGS += -Wall -Werror
    CFLAGS += -Wno-format-zero-length
    CFLAGS += -Wdeclaration-after-statement
    CFLAGS += -Wpointer-arith
    CFLAGS += -Wstrict-prototypes
    CFLAGS += -Wold-style-declaration

-Peff

Jeff King <peff@peff.net> writes:

I think I have no-pointer-to-int-cast, old-style-definition and vla in addition to the above.

On Thu, Jan 15, 2015 at 12:36:00PM -0800, Junio C Hamano wrote:

Thanks, I added the latter two to my setup. But what is the purpose of turning off pointer-to-int warnings? It seems like those are a good indication of a sloppy construct (and AFAICT, we do not have any code which triggers on it).

-Peff

Jeff King <peff@peff.net> writes:

It probably a remnant from olden days; perhaps we used to have a code that stuffs a pointer value to an int field used as a hash key or something. As you said, there is no need for disabling that check in today's code.

Thanks for catching.