All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com,
	bcm-kernel-feedback-list@broadcom.com, lorenzo.pieralisi@arm.com,
	robh@kernel.org, bhelgaas@google.com,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long'
Date: Mon, 8 Nov 2021 02:34:13 +0100	[thread overview]
Message-ID: <YYh+ldT5wU2s0sWY@rocinante> (raw)
In-Reply-To: <e6d9da2112aab2939d1507b90962d07bfd735b4c.1636273671.git.christophe.jaillet@wanadoo.fr>

Hi Christophe!

[...]
> This bitmap can be BRCM_INT_PCI_MSI_LEGACY_NR or BRCM_INT_PCI_MSI_NR long.

Ahh.  OK.  Given this an option would be to: do nothing (keep current
status quo); allocate memory dynamically passing the "msi->nr" after it
has been set accordingly; use BRCM_INT_PCI_MSI_NR and waste a little bit
of space.

Perhaps moving to using the DECLARE_BITMAP() would be fine in this case
too, at least to match style of other drivers more closely.

Jim, Florian and Lorenzo - is this something that would be OK with you,
or you would rather keep things as they were?

> Addresses-Coverity: "Out-of-bounds access (ARRAY_VS_SINGLETON)"

This tag would have to be written as:

  Addresses-Coverity: ("Out-of-bounds access (ARRAY_VS_SINGLETON)")

[...]
> +	DECLARE_BITMAP		(used, BRCM_INT_PCI_MSI_NR);

Probably not the most elegant solution, but I would keep it as:

  DECLARE_BITMAP(used, BRCM_INT_PCI_MSI_NR);

Otherwise aligning either before or after the open bracket will cause
either an error or a warning issued by checkpatch.pl accordingly about
the style.  Other users of this (a vast majoirty) macro don't do any
specific alignment at large

[...]
> +	/*
> +	 * Sanity check to make sure that the 'used' bitmap in struct brcm_msi
> +	 * is large enough.
> +	 */
> +	BUILD_BUG_ON(BRCM_INT_PCI_MSI_LEGACY_NR > BRCM_INT_PCI_MSI_NR);

A healthy paranoia, I see. :-)

	Krzysztof

WARNING: multiple messages have this Message-ID (diff)
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com,
	bcm-kernel-feedback-list@broadcom.com, lorenzo.pieralisi@arm.com,
	robh@kernel.org, bhelgaas@google.com,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long'
Date: Mon, 8 Nov 2021 02:34:13 +0100	[thread overview]
Message-ID: <YYh+ldT5wU2s0sWY@rocinante> (raw)
In-Reply-To: <e6d9da2112aab2939d1507b90962d07bfd735b4c.1636273671.git.christophe.jaillet@wanadoo.fr>

Hi Christophe!

[...]
> This bitmap can be BRCM_INT_PCI_MSI_LEGACY_NR or BRCM_INT_PCI_MSI_NR long.

Ahh.  OK.  Given this an option would be to: do nothing (keep current
status quo); allocate memory dynamically passing the "msi->nr" after it
has been set accordingly; use BRCM_INT_PCI_MSI_NR and waste a little bit
of space.

Perhaps moving to using the DECLARE_BITMAP() would be fine in this case
too, at least to match style of other drivers more closely.

Jim, Florian and Lorenzo - is this something that would be OK with you,
or you would rather keep things as they were?

> Addresses-Coverity: "Out-of-bounds access (ARRAY_VS_SINGLETON)"

This tag would have to be written as:

  Addresses-Coverity: ("Out-of-bounds access (ARRAY_VS_SINGLETON)")

[...]
> +	DECLARE_BITMAP		(used, BRCM_INT_PCI_MSI_NR);

Probably not the most elegant solution, but I would keep it as:

  DECLARE_BITMAP(used, BRCM_INT_PCI_MSI_NR);

Otherwise aligning either before or after the open bracket will cause
either an error or a warning issued by checkpatch.pl accordingly about
the style.  Other users of this (a vast majoirty) macro don't do any
specific alignment at large

[...]
> +	/*
> +	 * Sanity check to make sure that the 'used' bitmap in struct brcm_msi
> +	 * is large enough.
> +	 */
> +	BUILD_BUG_ON(BRCM_INT_PCI_MSI_LEGACY_NR > BRCM_INT_PCI_MSI_NR);

A healthy paranoia, I see. :-)

	Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-11-08  1:34 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-07  8:32 [PATCH] PCI: brcmstb: Declare a bitmap as a bitmap, not as a plain 'unsigned long' Christophe JAILLET
2021-11-07  8:32 ` Christophe JAILLET
2021-11-08  1:34 ` Krzysztof Wilczyński [this message]
2021-11-08  1:34   ` Krzysztof Wilczyński
2021-11-08 16:28   ` Florian Fainelli
2021-11-08 16:28     ` Florian Fainelli
2021-11-08 19:51     ` Christophe JAILLET
2021-11-08 19:51       ` Christophe JAILLET
2021-11-08 23:30       ` Krzysztof Wilczyński
2021-11-08 23:30         ` Krzysztof Wilczyński
2021-11-08 23:45         ` Florian Fainelli
2021-11-08 23:45           ` Florian Fainelli
2021-11-08 23:55           ` Krzysztof Wilczyński
2021-11-08 23:55             ` Krzysztof Wilczyński
2021-11-08 23:44 ` Florian Fainelli
2021-11-08 23:44   ` Florian Fainelli
2021-11-29 13:11 ` Lorenzo Pieralisi
2021-11-29 13:11   ` Lorenzo Pieralisi
2021-11-30 17:04 ` Bjorn Helgaas
2021-11-30 17:04   ` Bjorn Helgaas
2021-12-03 11:02   ` Dan Carpenter
2021-12-03 11:02     ` Dan Carpenter

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=YYh+ldT5wU2s0sWY@rocinante \
    --to=kw@linux.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=f.fainelli@gmail.com \
    --cc=jim2101024@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=nsaenz@kernel.org \
    --cc=robh@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.