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

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 27, 2021, 02:36 UTC
Message-ID: <YBDRoEzRKO6GIVCb@nand.local>
In-Reply-To: <CAPx1GvfQ4K+2mt0BC-1o7e0omHAvC6XhmpOuGa41v-b850w14w@mail.gmail.com>

On Tue, Jan 26, 2021 at 05:53:39PM -0800, Chris Torek wrote:

Show 15 quoted lines

> Note: this is purely style, and minor, but I'll ask...
>
> On Tue, Jan 26, 2021 at 8:08 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> 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