From: Stephen Boyd Date: Mon, 21 Mar 2011 22:04:32 GMT Subject: Re: [PATCHv2 2/2] Fix sparse warnings Message-ID: In-Reply-To: <7vd3lknnjy.fsf@alter.siamese.dyndns.org> On Mon, Mar 21, 2011 at 2:58 PM, Junio C Hamano wrote: > Junio C Hamano 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 >  #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.