Re: [PATCH][RFC] kernel.h: provide array iterator
On 2018-03-15 11:00, Kieran Bingham wrote: > Simplify array iteration with a helper to iterate each entry in an array. > Utilise the existing ARRAY_SIZE macro to identify the length of the array > and pointer arithmetic to process each item as a for loop. > > Signed-off-by: Kieran Bingham > --- > include/linux/kernel.h | 10 ++ > 1 file changed, 10 insertions(+) > > The use of static arrays to store data is a common use case throughout the > kernel. Along with that is the obvious need to iterate that data. > > In fact there are just shy of 5000 instances of iterating a static array: > git grep "for .*ARRAY_SIZE" | wc -l > 4943 > > When working on the UVC driver - I found that I needed to split one such > iteration into two parts, and at the same time felt that this could be > refactored to be cleaner / easier to read. About that, it would be helpful if you first converted to the new iterator, so that one can more easily see they are equivalent. And then split in two, adding the flush_workqueue call. Or do it the other way around. But please don't mix the two in one patch, especially not if it's supposed to act as an example of how to use the new helper. > I do however worry that this simple short patch might not be desired or could > also be heavily bikeshedded due to it's potential wide spread use (though > perhaps that would be a good thing to have more users) ... but here it is, > along with an example usage below which is part of a separate series. I think it can be useful, and it does have the must_be_array protection built in, so code doesn't silently break if one changes from a fixed-size allocation to e.g. a kmalloc-based one. Just don't attempt a tree-wide mass conversion, but obviously starting to make use of it when refactoring code anyway is fine. And now, the bikeshedding you expected :) > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index ce51455e2adf..95d7dae248b7 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -70,6 +70,16 @@ > */ > #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + > __must_be_array(arr)) > > +/** > + * for_each_array_element - Iterate all items in an array > + * @elem: pointer of array type for iteration cursor Hm, "pointer of array type" sounds wrong; it's not a "pointer to array". But "pointer of array elements' type" is clumsy. Maybe just "@elem: iteration cursor" is clear enough. > + * @array: array to be iterated > + */ > +#define for_each_array_element(elem, array) \ > + for (elem = &(array)[0]; \ > + elem < &(array)[ARRAY_SIZE(array)]; \ > + ++elem) > + Please parenthesize elem as well. Rasmus
[PATCH 2/4] [media] lmedm04: change some static variables to automatic
ibuf and rbuf in lme2510_int_response are always assigned to before they
are read, and their addresses do not escape the function, so they have
no reason to be static.
Signed-off-by: Rasmus Villemoes
---
drivers/media/usb/dvb-usb-v2/lmedm04.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c
b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 7692701878ba..cd463f09ebc7 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -315,7 +315,7 @@ static void lme2510_int_response(struct urb *lme_urb)
{
struct dvb_usb_adapter *adap = lme_urb->context;
struct lme2510_state *st = adap_to_priv(adap);
- static u8 *ibuf, *rbuf;
+ u8 *ibuf, *rbuf;
int i = 0, offset;
u32 key;
u8 signal_lock = 0;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] [media] lmedm04: make some string arrays static
It takes more .text to initialize these on the stack than they occupy
in .rodata, so just make them static const.
Signed-off-by: Rasmus Villemoes
---
drivers/media/usb/dvb-usb-v2/lmedm04.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c
b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index cd463f09ebc7..bf5bc36a6ed9 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -1002,8 +1002,9 @@ static int lme_name(struct dvb_usb_adapter *adap)
struct dvb_usb_device *d = adap_to_d(adap);
struct lme2510_state *st = adap_to_priv(adap);
const char *desc = d->name;
- char *fe_name[] = {"", " LG TDQY-P001F", " SHARP:BS2F7HZ7395",
- " SHARP:BS2F7HZ0194", " RS2000"};
+ static const char * const fe_name[] = {
+ "", " LG TDQY-P001F", " SHARP:BS2F7HZ7395",
+ " SHARP:BS2F7HZ0194", " RS2000"};
char *name = adap->fe[0]->ops.info.name;
strlcpy(name, desc, 128);
@@ -1124,7 +1125,7 @@ static int dm04_lme2510_tuner(struct dvb_usb_adapter
*adap)
{
struct dvb_usb_device *d = adap_to_d(adap);
struct lme2510_state *st = adap_to_priv(adap);
- char *tun_msg[] = {"", "TDA8263", "IX2505V", "DVB_PLL_OPERA", "RS2000"};
+ static const char * const tun_msg[] = {"", "TDA8263", "IX2505V",
"DVB_PLL_OPERA", "RS2000"};
int ret = 0;
switch (st->tuner_config) {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] [media] lmedm04: use %phN for hex dump
Using the %ph printf extension for hex dumps like this makes the generated code quite a bit smaller. Signed-off-by: Rasmus Villemoes --- drivers/media/usb/dvb-usb-v2/lmedm04.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c index 0e8fb89896c4..7692701878ba 100644 --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c @@ -99,9 +99,7 @@ static int dvb_usb_lme2510_debug; } while (0) #define deb_info(level, args...) lme_debug(dvb_usb_lme2510_debug, level, args) #define debug_data_snipet(level, name, p) \ -deb_info(level, name" (%02x%02x%02x%02x%02x%02x%02x%02x)", \ - *p, *(p+1), *(p+2), *(p+3), *(p+4), \ - *(p+5), *(p+6), *(p+7)); +deb_info(level, name" (%8phN)", p); #define info(args...) pr_info(DVB_USB_LOG_PREFIX": "args) module_param_named(debug, dvb_usb_lme2510_debug, int, 0644); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] [media] lmedm04: make lme2510_powerup a little smaller
gcc isn't smart enough to realize it can share most of the argument buildup and the actual function call between the two branches, so help it a little. Signed-off-by: Rasmus Villemoes --- drivers/media/usb/dvb-usb-v2/lmedm04.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c index bf5bc36a6ed9..7462207f4fd7 100644 --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c @@ -1179,10 +1179,7 @@ static int lme2510_powerup(struct dvb_usb_device *d, int onoff) mutex_lock(&d->i2c_mutex); - if (onoff) - ret = lme2510_usb_talk(d, lnb_on, len, rbuf, rlen); - else - ret = lme2510_usb_talk(d, lnb_off, len, rbuf, rlen); + ret = lme2510_usb_talk(d, onoff ? lnb_on : lnb_off, len, rbuf, rlen); st->i2c_talk_onoff = 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 4/7] drivers/media/pci/zoran: avoid fragile snprintf use
Appending to a string by doing snprintf(buf, bufsize, "%s...", buf,
...) is not guaranteed to work.
Signed-off-by: Rasmus Villemoes
---
drivers/media/pci/zoran/videocodec.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/pci/zoran/videocodec.c
b/drivers/media/pci/zoran/videocodec.c
index c01071635290..13a3c07cd259 100644
--- a/drivers/media/pci/zoran/videocodec.c
+++ b/drivers/media/pci/zoran/videocodec.c
@@ -116,8 +116,9 @@ videocodec_attach (struct videocodec_master *master)
goto out_module_put;
}
- snprintf(codec->name, sizeof(codec->name),
-"%s[%d]", codec->name, h->attached);
+ res = strlen(codec->name);
+ snprintf(codec->name + res, sizeof(codec->name) - res,
+"[%d]", h->attached);
codec->master_data = master;
res = codec->setup(codec);
if (res == 0) {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 6/7] [media] ati_remote: avoid fragile snprintf use
Passing overlapping source and destination to snprintf is fragile. Replace with a single (mostly) equivalent call. If one wants to preserve the space preceding udev->product whether or not there was a manufacturer, just remove udev->manufacturer from the && expression. Signed-off-by: Rasmus Villemoes --- drivers/media/rc/ati_remote.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c index a35631891cc0..b6d367ba128a 100644 --- a/drivers/media/rc/ati_remote.c +++ b/drivers/media/rc/ati_remote.c @@ -866,13 +866,10 @@ static int ati_remote_probe(struct usb_interface *interface, strlcat(ati_remote->rc_phys, "/input0", sizeof(ati_remote->rc_phys)); strlcat(ati_remote->mouse_phys, "/input1", sizeof(ati_remote->mouse_phys)); - if (udev->manufacturer) - strlcpy(ati_remote->rc_name, udev->manufacturer, - sizeof(ati_remote->rc_name)); - - if (udev->product) - snprintf(ati_remote->rc_name, sizeof(ati_remote->rc_name), -"%s %s", ati_remote->rc_name, udev->product); + snprintf(ati_remote->rc_name, sizeof(ati_remote->rc_name), "%s%s%s", + udev->manufacturer ?: "", + udev->manufacturer && udev->product ? " " : "", + udev->product ?: ""); if (!strlen(ati_remote->rc_name)) snprintf(ati_remote->rc_name, sizeof(ati_remote->rc_name), -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] exynos4-is: fix a format string bug
Ironically, 7d4020c3c400 ("[media] exynos4-is: fix some warnings when
compiling on arm64") fixed some format string bugs but introduced a
new one. buf_index is a simple int, so it should be printed with %d,
not %pad (which is correctly used for dma_addr_t).
Fixes: 7d4020c3c400 ("[media] exynos4-is: fix some warnings when compiling on
arm64")
Signed-off-by: Rasmus Villemoes
---
drivers/media/platform/exynos4-is/fimc-isp-video.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c
b/drivers/media/platform/exynos4-is/fimc-isp-video.c
index 6e6648446f00..337d49b4d103 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
@@ -221,8 +221,8 @@ static void isp_video_capture_buffer_queue(struct
vb2_buffer *vb)
ivb->dma_addr[i];
isp_dbg(2, &video->ve.vdev,
- "dma_buf %pad (%d/%d/%d) addr: %pad\n",
- &buf_index, ivb->index, i, vb->index,
+ "dma_buf %d (%d/%d/%d) addr: %pad\n",
+ buf_index, ivb->index, i, vb->index,
&ivb->dma_addr[i]);
}
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] drxd: use kzalloc in drxd_attach()
This saves a little .text and removes the sizeof(...) style
inconsistency. Use sizeof(*state) in accordance with CodingStyle.
Signed-off-by: Rasmus Villemoes
---
drivers/media/dvb-frontends/drxd_hard.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/drxd_hard.c
b/drivers/media/dvb-frontends/drxd_hard.c
index 34b9441840da..445a15c2714f 100644
--- a/drivers/media/dvb-frontends/drxd_hard.c
+++ b/drivers/media/dvb-frontends/drxd_hard.c
@@ -2950,10 +2950,9 @@ struct dvb_frontend *drxd_attach(const struct
drxd_config *config,
{
struct drxd_state *state = NULL;
- state = kmalloc(sizeof(struct drxd_state), GFP_KERNEL);
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
return NULL;
- memset(state, 0, sizeof(*state));
state->ops = drxd_ops;
state->dev = dev;
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] s5p_mfc: Remove redundant casts
Both sides of these assignments actually have type "const struct
vb2_mem_ops *", so the casts are unnecessary and slightly confusing.
Signed-off-by: Rasmus Villemoes
---
drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 165bc86..8daf291 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -815,7 +815,7 @@ static int s5p_mfc_open(struct file *file)
ret = -ENOENT;
goto err_queue_init;
}
- q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+ q->mem_ops = &vb2_dma_contig_memops;
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
ret = vb2_queue_init(q);
if (ret) {
@@ -837,7 +837,7 @@ static int s5p_mfc_open(struct file *file)
ret = -ENOENT;
goto err_queue_init;
}
- q->mem_ops = (struct vb2_mem_ops *)&vb2_dma_contig_memops;
+ q->mem_ops = &vb2_dma_contig_memops;
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
ret = vb2_queue_init(q);
if (ret) {
--
2.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drivers: media: i2c: adv7343_regs.h: Fix typo in #ifndef
Test for definedness of the macro which is actually defined, and which
matches the name of the file.
Signed-off-by: Rasmus Villemoes
---
drivers/media/i2c/adv7343_regs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv7343_regs.h b/drivers/media/i2c/adv7343_regs.h
index 4466067..2f04ce4 100644
--- a/drivers/media/i2c/adv7343_regs.h
+++ b/drivers/media/i2c/adv7343_regs.h
@@ -13,7 +13,7 @@
* GNU General Public License for more details.
*/
-#ifndef ADV7343_REG_H
+#ifndef ADV7343_REGS_H
#define ADV7343_REGS_H
struct adv7343_std_info {
--
2.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drivers: media: b2c2: flexcop.h: Fix typo in include guard
Three trailing underscores is one too many. Signed-off-by: Rasmus Villemoes --- drivers/media/common/b2c2/flexcop.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/common/b2c2/flexcop.h b/drivers/media/common/b2c2/flexcop.h index 897b10c..8942bda 100644 --- a/drivers/media/common/b2c2/flexcop.h +++ b/drivers/media/common/b2c2/flexcop.h @@ -4,7 +4,7 @@ * see flexcop.c for copyright information */ #ifndef __FLEXCOP_H__ -#define __FLEXCOP_H___ +#define __FLEXCOP_H__ #define FC_LOG_PREFIX "b2c2-flexcop" #include "flexcop-common.h" -- 2.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] staging: omap4iss: Fix type of struct iss_device::crashed
The crashed member of struct iss_device is documented to be a bitmask,
but a bool doesn't hold that many (usable) bits. Lines 589 and 659 of
iss.c strongly suggest that "unsigned int" was meant (the same type as
struct iss_pipeline::entities). Currently, any crashed entity will be
blamed on index 0, which is unlikely to be what was intended.
Signed-off-by: Rasmus Villemoes
---
drivers/staging/media/omap4iss/iss.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/omap4iss/iss.h
b/drivers/staging/media/omap4iss/iss.h
index 05cd9bf..734cfee 100644
--- a/drivers/staging/media/omap4iss/iss.h
+++ b/drivers/staging/media/omap4iss/iss.h
@@ -97,7 +97,7 @@ struct iss_device {
u64 raw_dmamask;
struct mutex iss_mutex; /* For handling ref_count field */
- bool crashed;
+ unsigned int crashed;
int has_context;
int ref_count;
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
