From: Jeff King <peff@peff.net>
Date: Thu, 15 Jan 2015 10:55:59 GMT
Subject: Re: [PATCH] init-db: remove unused #includes
Message-ID: <20150115105558.GA13536@peff.net>
In-Reply-To: <CANCZXo5UY80327pvjzYkJXroH_WZdW36LhSANK0O7GGVQLF=AA@mail.gmail.com>

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

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

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