しらいしななこ <nanako3@xxxxxxxxxxx> writes:
> These function are not used anywhere. Also removes graph_release()
> that is never called.
>
> Signed-off-by: Nanako Shiraishi <nanako3@xxxxxxxxxxx>
I CCed Adam, who is the primary author in this area.
> ---
> graph.c | 57 +++++++++++++++++++++++++++++++++++++++++++--------------
> graph.h | 40 ----------------------------------------
> 2 files changed, 43 insertions(+), 54 deletions(-)
>
> diff --git a/graph.c b/graph.c
> index e2633f8..5f82170 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -4,6 +4,43 @@
> #include "diff.h"
> #include "revision.h"
>
> +/* Internal API */
> + ...
> +static int graph_next_line(struct git_graph *graph, struct strbuf *sb);
> +static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
> +static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);
I think these are probably fine, not in the sense that nobody calls these
functions _right now_ but in the sense that I do not foresee a calling
sequence outside the graph.c internal that needs to call these directly,
instead of calling graph_show_*() functions that use these.
> @@ -180,14 +217,6 @@ struct git_graph *graph_init(struct rev_info *opt)
> return graph;
> }
>
> -void graph_release(struct git_graph *graph)
> -{
> - free(graph->columns);
> - free(graph->new_columns);
> - free(graph->mapping);
> - free(graph);
> -}
But I do not think this is right. The current lack of caller of this
clean-up function simply means the current users are leaking. I think
they are all of "set up rev_info, do a lengthy operation and exit" pattern
and clean-up immediately before exit is often omitted as unnecessary, but
if we had a clean-up function for the revision API that function would
call this one. I'd rather leave this in place, and let libification
minded people figure out the cleanest places and ways to make this
called.
Other three clean-ups looked Ok to me. Thanks.
--
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
