On Tue, 30 Mar 2021 at 17:08, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Tue, Mar 30, 2021 at 5:06 AM Ciara Loftus <ciara.loftus@xxxxxxxxx> wrote:
> >
[...]
> > if (--ctx->refcount == 0) {
> > - err = xsk_get_mmap_offsets(umem->fd, &off);
> > - if (!err) {
> > - munmap(ctx->fill->ring - off.fr.desc,
> > - off.fr.desc + umem->config.fill_size *
> > - sizeof(__u64));
> > - munmap(ctx->comp->ring - off.cr.desc,
> > - off.cr.desc + umem->config.comp_size *
> > - sizeof(__u64));
> > + if (unmap) {
> > + err = xsk_get_mmap_offsets(umem->fd, &off);
> > + if (!err) {
> > + munmap(ctx->fill->ring - off.fr.desc,
> > + off.fr.desc + umem->config.fill_size *
> > + sizeof(__u64));
> > + munmap(ctx->comp->ring - off.cr.desc,
> > + off.cr.desc + umem->config.comp_size *
> > + sizeof(__u64));
> > + }
>
> The whole function increases indent, since it changes anyway
> could you write it as:
> {
> if (--ctx->refcount)
> return;
> if (!unmap)
> goto out_free;
> err = xsk_get
> if (err)
> goto out_free;
> munmap();
> out_free:
> list_del
> free
> }
>
Yes, please try to reduce the nesting, and while at it try to expand
the as much as possible of the munmap arguments to the full 100 chars.
Björn
