* [PATCH] init-db: remove unused #includes
@ 2015-01-14 17:59 Alexander Kuleshov
2015-01-14 19:09 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Kuleshov @ 2015-01-14 17:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alexander Kuleshov
* "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(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 280454a..2978b36 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -3,9 +3,7 @@
*
* Copyright (C) Linus Torvalds, 2005
*/
-#include "cache.h"
#include "builtin.h"
-#include "exec_cmd.h"
#include "parse-options.h"
#ifndef DEFAULT_GIT_TEMPLATE_DIR
--
2.3.0.rc0.255.g53b80d0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-14 17:59 [PATCH] init-db: remove unused #includes Alexander Kuleshov
@ 2015-01-14 19:09 ` Junio C Hamano
2015-01-14 20:14 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2015-01-14 19:09 UTC (permalink / raw)
To: Alexander Kuleshov; +Cc: git
Alexander Kuleshov <kuleshovmail@gmail.com> writes:
> * "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>
> ---
Makes sense. Thanks.
> builtin/init-db.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 280454a..2978b36 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -3,9 +3,7 @@
> *
> * Copyright (C) Linus Torvalds, 2005
> */
> -#include "cache.h"
> #include "builtin.h"
> -#include "exec_cmd.h"
> #include "parse-options.h"
>
> #ifndef DEFAULT_GIT_TEMPLATE_DIR
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-14 19:09 ` Junio C Hamano
@ 2015-01-14 20:14 ` Junio C Hamano
2015-01-15 10:13 ` Alexander Kuleshov
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2015-01-14 20:14 UTC (permalink / raw)
To: Alexander Kuleshov; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Alexander Kuleshov <kuleshovmail@gmail.com> writes:
>
>> * "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>
>> ---
>
> Makes sense. Thanks.
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-14 20:14 ` Junio C Hamano
@ 2015-01-15 10:13 ` Alexander Kuleshov
2015-01-15 10:55 ` Jeff King
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Kuleshov @ 2015-01-15 10:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hello Junio,
yes right, missed system_path usage. But it's strange, code still
compiles successfully without exec_cmd.h.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-15 10:13 ` Alexander Kuleshov
@ 2015-01-15 10:55 ` Jeff King
2015-01-15 20:36 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2015-01-15 10:55 UTC (permalink / raw)
To: Alexander Kuleshov; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-15 10:55 ` Jeff King
@ 2015-01-15 20:36 ` Junio C Hamano
2015-01-15 22:31 ` Jeff King
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2015-01-15 20:36 UTC (permalink / raw)
To: Jeff King; +Cc: Alexander Kuleshov, git
Jeff King <peff@peff.net> writes:
> 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
I think I have no-pointer-to-int-cast, old-style-definition and vla
in addition to the above.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-15 20:36 ` Junio C Hamano
@ 2015-01-15 22:31 ` Jeff King
2015-01-15 23:40 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2015-01-15 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alexander Kuleshov, git
On Thu, Jan 15, 2015 at 12:36:00PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > 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
>
> I think I have no-pointer-to-int-cast, old-style-definition and vla
> in addition to the above.
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] init-db: remove unused #includes
2015-01-15 22:31 ` Jeff King
@ 2015-01-15 23:40 ` Junio C Hamano
0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2015-01-15 23:40 UTC (permalink / raw)
To: Jeff King; +Cc: Alexander Kuleshov, git
Jeff King <peff@peff.net> writes:
> On Thu, Jan 15, 2015 at 12:36:00PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > 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
>>
>> I think I have no-pointer-to-int-cast, old-style-definition and vla
>> in addition to the above.
>
> 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).
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-01-15 23:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 17:59 [PATCH] init-db: remove unused #includes Alexander Kuleshov
2015-01-14 19:09 ` Junio C Hamano
2015-01-14 20:14 ` Junio C Hamano
2015-01-15 10:13 ` Alexander Kuleshov
2015-01-15 10:55 ` Jeff King
2015-01-15 20:36 ` Junio C Hamano
2015-01-15 22:31 ` Jeff King
2015-01-15 23:40 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.