Subject: Re: [PATCH 01/17] commit-graph: anonymize data in chunk_write_fn

From: Taylor Blau <[email-masked]>
Date: Tue, 26 Jan 2021 21:36:16 -0500

On Tue, Jan 26, 2021 at 05:53:39PM -0800, Chris Torek wrote:
> Note: this is purely style, and minor, but I'll ask...
>
> On Tue, Jan 26, 2021 at 8:08 AM Derrick Stolee via GitGitGadget
> <[email-masked]> wrote:
> >  static int write_graph_chunk_fanout(struct hashfile *f,
> > -                                   struct write_commit_graph_context *ctx)
> > +                                   void *data)
> >  {
> > +       struct write_commit_graph_context *ctx =
> > +               (struct write_commit_graph_context *)data;
>
> Why bother with the cast on the last line here?  In C,
> conversion from `void *` to `struct whatever *` is fine.
>
> (the change itself looks fine, btw)

Agreed. It's not a correctness issue, but I find these unnecessary casts
to detract from readability. If you do end up rerolling this series,
I'd rather see

    struct write_commit_graph_context *ctx = data;

...but I don't think that this (non-)issue alone is worth a reroll.

Thanks,
Taylor