[alsa-devel] [PATCH 00/18] use ARRAY_SIZE macro
Hi everyone, Using ARRAY_SIZE improves the code readability. I used coccinelle (I made a change to the array_size.cocci file [1]) to find several places where ARRAY_SIZE could be used instead of other macros or sizeof division.
I tried to divide the changes into a patch per subsystem (excepted for staging). If one of the patch should be split into several patches, let me know.
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
This series is based on linux-next next-20170929. Each patch has been tested by building the relevant files with W=1.
This series contains the following patches: [PATCH 01/18] sound: use ARRAY_SIZE [PATCH 02/18] tracing/filter: use ARRAY_SIZE [PATCH 03/18] media: use ARRAY_SIZE [PATCH 04/18] IB/mlx5: Use ARRAY_SIZE [PATCH 05/18] net: use ARRAY_SIZE [PATCH 06/18] drm: use ARRAY_SIZE [PATCH 07/18] scsi: bfa: use ARRAY_SIZE [PATCH 08/18] ecryptfs: use ARRAY_SIZE [PATCH 09/18] nfsd: use ARRAY_SIZE [PATCH 10/18] orangefs: use ARRAY_SIZE [PATCH 11/18] dm space map metadata: use ARRAY_SIZE [PATCH 12/18] x86: use ARRAY_SIZE [PATCH 13/18] tpm: use ARRAY_SIZE [PATCH 14/18] ipmi: use ARRAY_SIZE [PATCH 15/18] acpi: use ARRAY_SIZE [PATCH 16/18] media: staging: atomisp: use ARRAY_SIZE [PATCH 17/18] staging: rtl8723bs: use ARRAY_SIZE [PATCH 18/18] staging: rtlwifi: use ARRAY_SIZE
Using the ARRAY_SIZE macro improves the readability of the code.
Found with Coccinelle with the following semantic patch: @r depends on (org || report)@ type T; T[] E; position p; @@ ( (sizeof(E)@p /sizeof(*E)) | (sizeof(E)@p /sizeof(E[...])) | (sizeof(E)@p /sizeof(T)) )
Signed-off-by: Jérémy Lefaure jeremy.lefaure@lse.epita.fr --- sound/oss/ad1848.c | 7 ++++--- sound/pci/hda/patch_ca0132.c | 8 +++----- 2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/sound/oss/ad1848.c b/sound/oss/ad1848.c index 2421f59cf279..7e495054b51c 100644 --- a/sound/oss/ad1848.c +++ b/sound/oss/ad1848.c @@ -49,6 +49,7 @@ #include <linux/isapnp.h> #include <linux/pnp.h> #include <linux/spinlock.h> +#include <linux/kernel.h>
#include "sound_config.h"
@@ -797,7 +798,7 @@ static int ad1848_set_speed(int dev, int arg)
int i, n, selected = -1;
- n = sizeof(speed_table) / sizeof(speed_struct); + n = ARRAY_SIZE(speed_table);
if (arg <= 0) return portc->speed; @@ -908,7 +909,7 @@ static unsigned int ad1848_set_bits(int dev, unsigned int arg) AFMT_U16_BE, 0 } }; - int i, n = sizeof(format2bits) / sizeof(struct format_tbl); + int i;
if (arg == 0) return portc->audio_format; @@ -918,7 +919,7 @@ static unsigned int ad1848_set_bits(int dev, unsigned int arg)
portc->audio_format = arg;
- for (i = 0; i < n; i++) + for (i = 0; i < ARRAY_SIZE(format2bits); i++) if (format2bits[i].format == arg) { if ((portc->format_bits = format2bits[i].bits) == 0) diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 3e73d5c6ccfc..768ea8651993 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -27,6 +27,7 @@ #include <linux/mutex.h> #include <linux/module.h> #include <linux/firmware.h> +#include <linux/kernel.h> #include <sound/core.h> #include "hda_codec.h" #include "hda_local.h" @@ -3605,8 +3606,7 @@ static int ca0132_vnode_switch_set(struct snd_kcontrol *kcontrol, static int ca0132_voicefx_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo) { - unsigned int items = sizeof(ca0132_voicefx_presets) - / sizeof(struct ct_voicefx_preset); + unsigned int items = ARRAY_SIZE(ca0132_voicefx_presets);
uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; uinfo->count = 1; @@ -3635,10 +3635,8 @@ static int ca0132_voicefx_put(struct snd_kcontrol *kcontrol, struct ca0132_spec *spec = codec->spec; int i, err = 0; int sel = ucontrol->value.enumerated.item[0]; - unsigned int items = sizeof(ca0132_voicefx_presets) - / sizeof(struct ct_voicefx_preset);
- if (sel >= items) + if (sel >= ARRAY_SIZE(ca0132_voicefx_presets)) return 0;
codec_dbg(codec, "ca0132_voicefx_put: sel=%d, preset=%s\n",
On Sun, 2017-10-01 at 15:30 -0400, Jérémy Lefaure wrote:
Using the ARRAY_SIZE macro improves the readability of the code.
Found with Coccinelle with the following semantic patch: @r depends on (org || report)@ type T; T[] E; position p; @@ ( (sizeof(E)@p /sizeof(*E))
(sizeof(E)@p /sizeof(E[...]))
(sizeof(E)@p /sizeof(T)) )
[]
diff --git a/sound/oss/ad1848.c b/sound/oss/ad1848.c
[]
@@ -797,7 +798,7 @@ static int ad1848_set_speed(int dev, int arg)
int i, n, selected = -1;
- n = sizeof(speed_table) / sizeof(speed_struct);
- n = ARRAY_SIZE(speed_table);
These sorts of changes are OK, but for many uses, it's more readable to use ARRAY_SIZE(foo) in each location rather than using a temporary.
On Sun, 01 Oct 2017 21:16:19 -0700 Joe Perches joe@perches.com wrote:
These sorts of changes are OK, but for many uses, it's more readable to use ARRAY_SIZE(foo) in each location rather than using a temporary.
You're right, I missed that one. I will send a v2.
Thank you for your review, Jérémy
On Sun, 01 Oct 2017 21:30:39 +0200, Jérémy Lefaure wrote:
Using the ARRAY_SIZE macro improves the readability of the code.
Found with Coccinelle with the following semantic patch: @r depends on (org || report)@ type T; T[] E; position p; @@ ( (sizeof(E)@p /sizeof(*E)) | (sizeof(E)@p /sizeof(E[...])) | (sizeof(E)@p /sizeof(T)) )
Signed-off-by: Jérémy Lefaure jeremy.lefaure@lse.epita.fr
sound/oss/ad1848.c | 7 ++++--- sound/pci/hda/patch_ca0132.c | 8 +++----- 2 files changed, 7 insertions(+), 8 deletions(-)
sound/oss is already dead and will be removed likely in the next release. So just keep it untouched.
thanks,
Takashi
On Sun, Oct 01, 2017 at 03:30:38PM -0400, Jérémy Lefaure wrote:
Hi everyone, Using ARRAY_SIZE improves the code readability. I used coccinelle (I made a change to the array_size.cocci file [1]) to find several places where ARRAY_SIZE could be used instead of other macros or sizeof division.
I tried to divide the changes into a patch per subsystem (excepted for staging). If one of the patch should be split into several patches, let me know.
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
Why don't you just send individual patches for each subsystem? I'm not a maintainer but I don't see how any one person is going to be able to apply this whole series, it is making it hard for maintainers if they have to pick patches out from among the series (if indeed any will bother doing that).
I get that this will be more work for you but AFAIK we are optimizing for maintainers.
Good luck, Tobin.
On Mon, 2 Oct 2017 09:01:31 +1100 "Tobin C. Harding" me@tobin.cc wrote:
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
Why don't you just send individual patches for each subsystem? I'm not a maintainer but I don't see how any one person is going to be able to apply this whole series, it is making it hard for maintainers if they have to pick patches out from among the series (if indeed any will bother doing that).
Yeah, maybe it would have been better to send individual patches.
From my point of view it's a series because the patches are related (I did a git format-patch from my local branch). But for the maintainers point of view, they are individual patches.
As each patch in this series is standalone, the maintainers can pick the patches they want and apply them individually. So yeah, maybe I should have sent individual patches. But I also wanted to tie all patches together as it's the same change.
Anyway, I can tell to each maintainer that they can apply the patches they're concerned about and next time I may send individual patches.
Thank you for your answer, Jérémy
On Sun, Oct 01, 2017 at 08:52:20PM -0400, Jérémy Lefaure wrote:
On Mon, 2 Oct 2017 09:01:31 +1100 "Tobin C. Harding" me@tobin.cc wrote:
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
Why don't you just send individual patches for each subsystem? I'm not a maintainer but I don't see how any one person is going to be able to apply this whole series, it is making it hard for maintainers if they have to pick patches out from among the series (if indeed any will bother doing that).
Yeah, maybe it would have been better to send individual patches.
From my point of view it's a series because the patches are related (I did a git format-patch from my local branch). But for the maintainers point of view, they are individual patches.
And the maintainers view is what matters here, if you wish to get your patches reviewed and accepted...
thanks,
greg k-h
On Mon, Oct 02, 2017 at 07:35:54AM +0200, Greg KH wrote:
On Sun, Oct 01, 2017 at 08:52:20PM -0400, Jérémy Lefaure wrote:
On Mon, 2 Oct 2017 09:01:31 +1100 "Tobin C. Harding" me@tobin.cc wrote:
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
Why don't you just send individual patches for each subsystem? I'm not a maintainer but I don't see how any one person is going to be able to apply this whole series, it is making it hard for maintainers if they have to pick patches out from among the series (if indeed any will bother doing that).
Yeah, maybe it would have been better to send individual patches.
From my point of view it's a series because the patches are related (I did a git format-patch from my local branch). But for the maintainers point of view, they are individual patches.
And the maintainers view is what matters here, if you wish to get your patches reviewed and accepted...
Mainly I'd just like to know which you're asking for. Do you want me to apply this, or to ACK it so someone else can? If it's sent as a series I tend to assume the latter.
But in this case I'm assuming it's the former, so I'll pick up the nfsd one....
--b.
On Mon, 2 Oct 2017 15:22:24 -0400 bfields@fieldses.org (J. Bruce Fields) wrote:
Mainly I'd just like to know which you're asking for. Do you want me to apply this, or to ACK it so someone else can? If it's sent as a series I tend to assume the latter.
But in this case I'm assuming it's the former, so I'll pick up the nfsd one....
Could you to apply the NFSD patch ("nfsd: use ARRAY_SIZE") with the Reviewed-by: Jeff Layton jlayton@redhat.com) tag please ?
This patch is an individual patch and it should not have been sent in a series (sorry about that).
Thank you, Jérémy
On Mon, Oct 02, 2017 at 09:33:12PM -0400, Jérémy Lefaure wrote:
On Mon, 2 Oct 2017 15:22:24 -0400 bfields@fieldses.org (J. Bruce Fields) wrote:
Mainly I'd just like to know which you're asking for. Do you want me to apply this, or to ACK it so someone else can? If it's sent as a series I tend to assume the latter.
But in this case I'm assuming it's the former, so I'll pick up the nfsd one....
Could you to apply the NFSD patch ("nfsd: use ARRAY_SIZE") with the Reviewed-by: Jeff Layton jlayton@redhat.com) tag please ?
This patch is an individual patch and it should not have been sent in a series (sorry about that).
Applying for 4.15, thanks.--b.
Em Sun, 1 Oct 2017 20:52:20 -0400 Jérémy Lefaure jeremy.lefaure@lse.epita.fr escreveu:
Anyway, I can tell to each maintainer that they can apply the patches they're concerned about and next time I may send individual patches.
In the case of media, we'll handle it as if they were individual ones.
Thanks, Mauro
Thanks for the patch! :)
2017-10-01 22:30 GMT+03:00 Jérémy Lefaure jeremy.lefaure@lse.epita.fr:
Hi everyone, Using ARRAY_SIZE improves the code readability. I used coccinelle (I made a change to the array_size.cocci file [1]) to find several places where ARRAY_SIZE could be used instead of other macros or sizeof division.
I tried to divide the changes into a patch per subsystem (excepted for staging). If one of the patch should be split into several patches, let me know.
In order to reduce the size of the To: and Cc: lines, each patch of the series is sent only to the maintainers and lists concerned by the patch. This cover letter is sent to every list concerned by this series.
This series is based on linux-next next-20170929. Each patch has been tested by building the relevant files with W=1.
This series contains the following patches: [PATCH 01/18] sound: use ARRAY_SIZE [PATCH 02/18] tracing/filter: use ARRAY_SIZE [PATCH 03/18] media: use ARRAY_SIZE [PATCH 04/18] IB/mlx5: Use ARRAY_SIZE [PATCH 05/18] net: use ARRAY_SIZE [PATCH 06/18] drm: use ARRAY_SIZE [PATCH 07/18] scsi: bfa: use ARRAY_SIZE [PATCH 08/18] ecryptfs: use ARRAY_SIZE [PATCH 09/18] nfsd: use ARRAY_SIZE [PATCH 10/18] orangefs: use ARRAY_SIZE [PATCH 11/18] dm space map metadata: use ARRAY_SIZE [PATCH 12/18] x86: use ARRAY_SIZE [PATCH 13/18] tpm: use ARRAY_SIZE [PATCH 14/18] ipmi: use ARRAY_SIZE [PATCH 15/18] acpi: use ARRAY_SIZE [PATCH 16/18] media: staging: atomisp: use ARRAY_SIZE [PATCH 17/18] staging: rtl8723bs: use ARRAY_SIZE [PATCH 18/18] staging: rtlwifi: use ARRAY_SIZE
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
participants (9)
-
bfields＠fieldses.org -
Greg KH -
J. Bruce Fields
-
Joe Perches -
Jérémy Lefaure -
Mauro Carvalho Chehab -
Takashi Iwai -
Tobin C. Harding -
Zhi Wang