netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Alex Elder <elder@linaro.org>
Cc: David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	evgreen@chromium.org,
	Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>,
	cpratapa@codeaurora.org, bjorn.andersson@linaro.org,
	Network Development <netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 5/5] net: ipa: avoid going past end of resource group array
Date: Wed, 28 Oct 2020 09:42:23 -0400	[thread overview]
Message-ID: <CAF=yD-LjW5OpKcT+CNPmSkFfDgSGoa2hsFqS9wkMzdNDG1_eRQ@mail.gmail.com> (raw)
In-Reply-To: <95d20d91-d187-2638-6978-8c0ff752b49f@linaro.org>

> >> +       /* We program at most 6 source or destination resource group limits */
> >> +       BUILD_BUG_ON(IPA_RESOURCE_GROUP_SRC_MAX > 6);
> >> +
> >>         group_count = ipa_resource_group_src_count(ipa->version);
> >> -       if (!group_count)
> >> +       if (!group_count || group_count >= IPA_RESOURCE_GROUP_SRC_MAX)
> >>                 return false;
> >
> > Perhaps more a comment to the previous patch, but _MAX usually denotes
> > the end of an inclusive range, here 5. The previous name COUNT better
> > reflects the number of elements in range [0, 5], which is 6.
>
> I agree with your point, but the max here represents something different
> from what you're expecting.
>
> For a given resource type (source or destination) there is some fixed
> number (count) of resources available based on the version of SoC.
> The *driver* can handle any number of them up to the maximum number
> (max) for any SoC it supports.  In that respect, it *does* represent
> the largest value in an inclusive range.
>
> I could change the suffix to something like SRC_COUNT_MAX, but in
> general the symbol names are longer than I like in this driver and
> I'm trying to shorten them where possible.

Makes sense. Can you then call this out more explicitly in the commit
message? That MAX here means max count, not max element id.

  reply	other threads:[~2020-10-28 23:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-27 16:11 [PATCH net 0/5] net: ipa: minor bug fixes Alex Elder
2020-10-27 16:11 ` [PATCH net 1/5] net: ipa: assign proper packet context base Alex Elder
2020-10-27 16:11 ` [PATCH net 2/5] net: ipa: fix resource group field mask definition Alex Elder
2020-10-27 16:11 ` [PATCH net 3/5] net: ipa: assign endpoint to a resource group Alex Elder
2020-10-27 16:11 ` [PATCH net 4/5] net: ipa: distinguish between resource group types Alex Elder
2020-10-27 16:11 ` [PATCH net 5/5] net: ipa: avoid going past end of resource group array Alex Elder
2020-10-28  0:14   ` Willem de Bruijn
2020-10-28 12:45     ` Alex Elder
2020-10-28 13:42       ` Willem de Bruijn [this message]
2020-10-28 13:30     ` Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF=yD-LjW5OpKcT+CNPmSkFfDgSGoa2hsFqS9wkMzdNDG1_eRQ@mail.gmail.com' \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=cpratapa@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=elder@linaro.org \
    --cc=evgreen@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=subashab@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).