Re: [PATCH] maple_tree: modified return type of mas_wr_store_entry()

[JaeJoon Jung]

Thank you so much for clarifying and correcting my comments and
sharing it with relevant groups.

On Tue, 28 May 2024 at 00:08, Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> wrote:
>
> * JaeJoon Jung <rgbi3307@xxxxxxxxx> [240524 21:02]:
> > From: Jung-JaeJoon <rgbi3307@xxxxxxxxx>
> >
>
> Thank you for your cleanup patch.
>
> I have added two more mailing lists to the Cc.  Please Cc them on any
> other patches related to the maple tree and mm area.
>
> > Since there is no place to use the return value of the mas_wr_store_entry() function,
> > it is correct to modify it to void.
>
> This comment isn't clear.
>
> Maybe something like (please, modify it to your liking):
> "Since the return value of mas_wr_store_entry() is not used, the return
> type can be changed to void"
>
> >
> > Signed-off-by: JaeJoon Jung <rgbi3307@xxxxxxxxx>
> > ---
> >  lib/maple_tree.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 2d7d27e6ae3c..da30977aab0f 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -4203,31 +4203,28 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas)
> >   *
> >   * Return: The contents that was stored at the index.
> >   */
> > -static inline void *mas_wr_store_entry(struct ma_wr_state *wr_mas)
> > +static inline void mas_wr_store_entry(struct ma_wr_state *wr_mas)
> >  {
> >       struct ma_state *mas = wr_mas->mas;
> >
> >       wr_mas->content = mas_start(mas);
> >       if (mas_is_none(mas) || mas_is_ptr(mas)) {
> >               mas_store_root(mas, wr_mas->entry);
> > -             return wr_mas->content;
> > +             return;
> >       }
> >
> >       if (unlikely(!mas_wr_walk(wr_mas))) {
> >               mas_wr_spanning_store(wr_mas);
> > -             return wr_mas->content;
> > +             return;
> >       }
> >
> >       /* At this point, we are at the leaf node that needs to be altered. */
> >       mas_wr_end_piv(wr_mas);
> >       /* New root for a single pointer */
> > -     if (unlikely(!mas->index && mas->last == ULONG_MAX)) {
> > +     if (unlikely(!mas->index && mas->last == ULONG_MAX))
> >               mas_new_root(mas, wr_mas->entry);
> > -             return wr_mas->content;
>
> Please keep this return statement, as you did above, for consistency and
> code clarity.
>
> > -     }
> > -
> > -     mas_wr_modify(wr_mas);
> > -     return wr_mas->content;
> > +        else
> > +             mas_wr_modify(wr_mas);
> >  }
> >
> >  /**
> > --
> > 2.17.1
> >