Re: [PATCHv2 2/2] Fix sparse warnings
- From
Stephen Boyd <bebarino@gmail.com>
- Date
- Mar 21, 2011, 22:04 UTC
- Message-ID
- <AANLkTinYCqK6zm17O_HedOFtbN6VRhYQbFj-YNk+JrV1@mail.gmail.com>
- In-Reply-To
- <7vd3lknnjy.fsf@alter.siamese.dyndns.org>
On Mon, Mar 21, 2011 at 2:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> Junio C Hamano <gitster@pobox.com> 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.
Show 37 quoted lines
> > 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.