On Mon, Mar 21, 2011 at 2:58 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Still yeek... >> >> What I meant was more like at the minimum: >> ... >> or much more preferably: >> >> Â- These files use symbols without declaring, because they do not include >>  Â"builtin.h": >> >>   builtin/clone.c (cmd_clone), builtin/fetch-pack.c (cmd_fetch_pack), ... >> >> Â- These files define extern symbols without declaring, and they can be >>  Âfile scope static: >> >>   builtin/fmt-merge-msg.c (init_src_data), ... >> >> Â- These callsites pass literal integer 0 where they mean to pass a NULL >>  Âpointer: >> >>  Âbuiltin/notes.c (resolve_ref), ... >> >> The patch text itself look more or less Ok, but I see you have builtin.h >> not as the first include in builtin/pack-redundant.c. >> Ah ok, I can do that. > > I spotted these two. Âthread-utils.h already includes pthread.h, and > builtin.h should come before (though technically exec_cmd.h does not > depend on any external types, so this is just a conformity issue, not > correctness one). > > Again, thanks. > > Âbuiltin/pack-redundant.c |  Â2 +- > Âthread-utils.c      |  Â1 - > Â2 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c > index 760b377..a15e366 100644 > --- a/builtin/pack-redundant.c > +++ b/builtin/pack-redundant.c > @@ -6,8 +6,8 @@ > Â* > Â*/ > > -#include "exec_cmd.h" > Â#include "builtin.h" > +#include "exec_cmd.h" > > Â#define BLKSIZE 512 > > diff --git a/thread-utils.c b/thread-utils.c > index 2c8c1e3..7f4b76a 100644 > --- a/thread-utils.c > +++ b/thread-utils.c > @@ -1,5 +1,4 @@ > Â#include "cache.h" > -#include <pthread.h> > Â#include "thread-utils.h" > > Â#if defined(hpux) || defined(__hpux) || defined(_hpux) > Ok, I'll squash these in and resend tonight when I get home. Also, I don't think exec_cmd.h is actually used in some of the builtin C files (due to some setup fallouts) so I think we can probably just remove the exec_cmd.h includes if they're within contex and unused. I'll do that next round. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html