Hi Ramsay,
On Fri, 4 May 2018, Ramsay Jones wrote:
> On 03/05/18 21:25, Johannes Schindelin wrote:
>
> > On Thu, 3 May 2018, Ramsay Jones wrote:
>
> >> On 03/05/18 16:30, Johannes Schindelin wrote:
> [snip]
>
> >>> diff --git a/builtin/branch-diff.c b/builtin/branch-diff.c
> >>> new file mode 100644
> >>> index 00000000000..97266cd326d
> >>> --- /dev/null
> >>> +++ b/builtin/branch-diff.c
> >>> @@ -0,0 +1,40 @@
> >>> +#include "cache.h"
> >>> +#include "parse-options.h"
> >>> +
> >>> +static const char * const builtin_branch_diff_usage[] = {
> >>> + N_("git rebase--helper [<options>] ( A..B C..D | A...B | base A B )"),
> >>
> >> s/rebase--helper/branch-diff/
> >
> > Whoops!
> >
> > BTW funny side note: when I saw that you replied, I instinctively thought
> > "oh no, I forgot to mark a function as `static`!" ;-)
>
> Heh, but I hadn't got around to applying the patches and building
> git yet! ;-)
;-)
> Sparse has two complaints:
>
> > SP builtin/branch-diff.c
> > builtin/branch-diff.c:433:41: warning: Using plain integer as NULL pointer
> > builtin/branch-diff.c:431:5: warning: symbol 'cmd_branch_diff' was not declared. Should it be static?
>
> I suppressed those warnings with the following patch (on top
> of these patches):
>
> $ git diff
> diff --git a/builtin/branch-diff.c b/builtin/branch-diff.c
> index edf80ecb7..1373c22f4 100644
> --- a/builtin/branch-diff.c
> +++ b/builtin/branch-diff.c
> @@ -1,4 +1,5 @@
> #include "cache.h"
> +#include "builtin.h"
> #include "parse-options.h"
> #include "string-list.h"
> #include "run-command.h"
> @@ -430,7 +431,7 @@ static void output(struct string_list *a, struct string_list *b,
>
> int cmd_branch_diff(int argc, const char **argv, const char *prefix)
> {
> - struct diff_options diffopt = { 0 };
> + struct diff_options diffopt = { NULL };
> struct strbuf four_spaces = STRBUF_INIT;
> int dual_color = 0;
> double creation_weight = 0.6;
> $
Thanks!
> The first hunk applies to patch 02/18 (ie this very patch) and
> the second hunk should be applied to patch 05/18 (ie, "branch-diff:
> also show the diff between patches").
I actually have a hacky script to fixup commits in a patch series. It lets
me stage part of the current changes, then figures out which of the
commits' changes overlap with the staged changed. If there is only one
commit, it automatically commits with --fixup, otherwise it lets me choose
which one I want to fixup (giving me the list of candidates).
BTW I ran `make sparse` for the first time, and it spits out tons of
stuff. And I notice that they are all non-fatal warnings, but so were the
ones you pointed out above. This is a bit sad, as I would *love* to
install a VSTS build job to run `make sparse` automatically. Examples of
warnings *after* applying your patch:
connect.c:481:40: warning: incorrect type in argument 2 (invalid types)
connect.c:481:40: expected union __CONST_SOCKADDR_ARG [usertype] __addr
connect.c:481:40: got struct sockaddr *ai_addr
or
pack-revindex.c:65:23: warning: memset with byte count of 262144
What gives?
Ciao,
Dscho
