On Sun, Jun 7, 2026 at 4:02 AM Leon Romanovsky <[email protected]> wrote:
>
> On Thu, Jun 04, 2026 at 03:36:48PM -0400, David Hu wrote:
> > On Thu, Jun 4, 2026 at 5:43 AM Leon Romanovsky <[email protected]> wrote:
> > >
> > > On Mon, Jun 01, 2026 at 08:00:12PM +0000, David Hu wrote:
> > > > @@ -133,6 +137,11 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct 
> > > > dma_buf_attachment *attach,
> > > >       }
> > > >
> > > >       nents = calc_sg_nents(dma->state, phys_vec, nr_ranges, size);
> > > > +     if (!nents) {
> > > > +             ret = -EINVAL;
> > > > +             goto err_free_state;
> > > > +     }
> > >
> > > Technically, this hunk is not necessary, since sg_alloc_table() will
> > > return -EINVAL when nents == 0. At least, that is the behavior I relied 
> > > on.
> >
> > I originally added this explicit check in v5 to address Jason's
> > feedback, and to make the
> > failure explicit rather than relying on `sg_alloc_table()` failing
> > silently on `nents=0`.
>
> I prefer explicit checks, but I am not in favor of duplicating them.
> Since sg_alloc_table() already validates this condition, we do not need
> to repeat the same check in dma-buf. A comment should be sufficient to
> inform future reviewers that nents == 0 is already handled.
>
> Thanks

Hi Leon,

Thank you for clarifying this further. Removing the duplication here
sounds good to
me. I'll drop the hunk, add a comment for posterity noting that `nents
= 0` is
handled by `sg_alloc_table()`, and send out a v6 shortly.

Thanks a bunch,
David

Reply via email to