[PATCH] udmabuf: fix odd_ptr_err.cocci warnings
From: kbuild test robotdrivers/dma-buf/udmabuf.c:167:6-12: inconsistent IS_ERR and PTR_ERR on line 168. PTR_ERR should access the value just tested by IS_ERR Semantic patch information: There can be false positives in the patch case, where it is the call to IS_ERR that is wrong. Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci Fixes: cc2d0e91bc15 ("udmabuf: driver update") Signed-off-by: kbuild test robot Signed-off-by: linux-ker...@vger.kernel.org --- tree: git://git.kraxel.org/linux udmabuf head: cc2d0e91bc15849baff695d175bfb8fba35f1465 commit: cc2d0e91bc15849baff695d175bfb8fba35f1465 [6/6] udmabuf: driver update udmabuf.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -165,7 +165,7 @@ static long udmabuf_ioctl_create(struct page = shmem_read_mapping_page( file_inode(ubuf->filp)->i_mapping, pgoff + pgidx); if (IS_ERR(page)) { - ret = PTR_ERR(buf); + ret = PTR_ERR(page); goto err_put_pages; } ubuf->pages[pgidx] = page;
Re: [PATCH 2/3] media: staging: atomisp: Fix an error handling path in 'lm3554_probe()'
On Fri, 11 May 2018, Christophe JAILLET wrote: > The use of 'fail1' and 'fail2' is not correct. Reorder these calls to > branch at the right place of the error handling path. Maybe it would be good to improve the names at the same time? julia > > Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr> > --- > drivers/staging/media/atomisp/i2c/atomisp-lm3554.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c > b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c > index 723fa74ff815..1e5f516f6e50 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c > @@ -871,7 +871,7 @@ static int lm3554_probe(struct i2c_client *client) >ARRAY_SIZE(lm3554_controls)); > if (err) { > dev_err(>dev, "error initialize a ctrl_handler.\n"); > - goto fail2; > + goto fail1; > } > > for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) > @@ -879,7 +879,6 @@ static int lm3554_probe(struct i2c_client *client) >NULL); > > if (flash->ctrl_handler.error) { > - > dev_err(>dev, "ctrl_handler error.\n"); > goto fail2; > } > @@ -888,7 +887,7 @@ static int lm3554_probe(struct i2c_client *client) > err = media_entity_pads_init(>sd.entity, 0, NULL); > if (err) { > dev_err(>dev, "error initialize a media entity.\n"); > - goto fail1; > + goto fail2; > } > > flash->sd.entity.function = MEDIA_ENT_F_FLASH; > -- > 2.17.0 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
[PATCH v2] [media] pvrusb2: delete unneeded include
pvrusb2-video-v4l.h only declares pvr2_saa7115_subdev_update and includes pvrusb2-hdw-internal.h. pvrusb2-cx2584x-v4l.c does not use pvr2_saa7115_subdev_update and it explicitly includes pvrusb2-hdw-internal.h. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- v2: Make the subject line a bit less generic drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c index 242b213..d5bec0f 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c @@ -23,7 +23,6 @@ */ #include "pvrusb2-cx2584x-v4l.h" -#include "pvrusb2-video-v4l.h" #include "pvrusb2-hdw-internal.h" -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] media: delete unneeded include
pvrusb2-video-v4l.h only declares pvr2_saa7115_subdev_update and includes pvrusb2-hdw-internal.h. pvrusb2-cx2584x-v4l.c does not use pvr2_saa7115_subdev_update and it explicitly includes pvrusb2-hdw-internal.h. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c index 242b213..d5bec0f 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c @@ -23,7 +23,6 @@ */ #include "pvrusb2-cx2584x-v4l.h" -#include "pvrusb2-video-v4l.h" #include "pvrusb2-hdw-internal.h"
Re: [PATCH][RFC] kernel.h: provide array iterator
Le 16.03.2018 05:21, Kees Cook a écrit : On Thu, Mar 15, 2018 at 3:00 AM, Kieran Bingham <kieran.bing...@ideasonboard.com> 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 <kieran.bing...@ideasonboard.com> --- 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 I suspect the main question is "Does this macro make the code easier to read?" I think it does, and we have other kinds of iterators like this in the kernel already. Would it be worth building a Coccinelle script to do the 5000 replacements? Coccinelle should be able to replace the for loop header. Coccinelle could create the local macro. Coccinelle might not put the definition in exactly the right place. Before the function of the first use would be possible, or before any function. I don't think that Coccinelle could figure out how to split one loop into two as done here, unless that specific pattern is very common. I guess that the split is to add the flush_workqueue, and is not the main goal? julia -Kees 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. 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. The aim is to simplify iteration on static arrays, in the same way that we have iterators for lists. The use of the ARRAY_SIZE macro, provides all the protections given by "__must_be_array(arr)" to this macro too. Regards Kieran = Example Usage from a pending UVC development: +#define for_each_uvc_urb(uvc_urb, uvc_streaming) \ + for_each_array_element(uvc_urb, uvc_streaming->uvc_urb) /* * Uninitialize isochronous/bulk URBs and free transfer buffers. */ static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers) { - struct urb *urb; - unsigned int i; + struct uvc_urb *uvc_urb; uvc_video_stats_stop(stream); - for (i = 0; i < UVC_URBS; ++i) { - struct uvc_urb *uvc_urb = >uvc_urb[i]; + for_each_uvc_urb(uvc_urb, stream) + usb_kill_urb(uvc_urb->urb); - urb = uvc_urb->urb; - if (urb == NULL) - continue; + flush_workqueue(stream->async_wq); - usb_kill_urb(urb); - usb_free_urb(urb); + for_each_uvc_urb(uvc_urb, stream) { + usb_free_urb(uvc_urb->urb); uvc_urb->urb = NULL; } if (free_buffers) uvc_free_urb_buffers(stream); } = 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 + * @array: array to be iterated + */ +#define for_each_array_element(elem, array) \ + for (elem = &(array)[0]; \ +elem < &(array)[ARRAY_SIZE(array)]; \ +++elem) + #define u64_to_user_ptr(x) ( \ { \ typecheck(u64, x); \ -- 2.7.4
Re: [Outreachy kernel] [PATCH 0/3] staging: media: cleanup
On Sun, 4 Mar 2018, Arushi Singhal wrote: > Spellcheck the comments. > Remove the repeated, consecutive words with single word. For the series: Acked-by: Julia Lawall <julia.law...@lip6.fr> But please look out for things to change in the code, not just in the comments. julia > > Arushi Singhal (3): > staging: media: Replace "be be" with "be" > staging: media: Replace "cant" with "can't" > staging: media: Replace "dont" with "don't" > > drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe_public.h | 2 > +- > drivers/staging/media/atomisp/pci/atomisp2/mmu/isp_mmu.c| 2 > +- > drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c| 2 > +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/1520178507-25141-1-git-send-email-arushisinghal19971997%40gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [Outreachy kernel] [PATCH] staging: media: Remove unnecessary semicolon
On Sun, 4 Mar 2018, Arushi Singhal wrote: > Remove unnecessary semicolon found using semicolon.cocci Coccinelle > script. > > Signed-off-by: Arushi Singhal <arushisinghal19971...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > --- > .../media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c| 2 > +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c > > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c > index 5faa89a..7562bea 100644 > --- > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c > +++ > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c > @@ -196,7 +196,7 @@ enum ia_css_err ia_css_frame_map(struct ia_css_frame > **frame, > attribute, context); > if (me->data == mmgr_NULL) > err = IA_CSS_ERR_INVALID_ARGUMENTS; > - }; > + } > > if (err != IA_CSS_SUCCESS) { > sh_css_free(me); > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20180303192629.GA5198%40seema-Inspiron-15-3567. > For more options, visit https://groups.google.com/d/optout. >
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Tue, 6 Feb 2018, Dan Carpenter wrote: > That found 4 that I think Wolfram's grep missed. > > arch/um/drivers/vector_user.h |2 -- > drivers/gpu/drm/mxsfb/mxsfb_regs.h|2 -- > drivers/video/fbdev/mxsfb.c |2 -- > include/drm/drm_scdc_helper.h |3 --- > > But it didn't find the two bugs that Geert found where the right side > wasn't a number literal. > > drivers/net/can/m_can/m_can.c:#define RXFC_FWM_MASK (0x7f < > RXFC_FWM_SHIFT) OK, I can easily add this in - I've got rules to protect against reporting it at the moment. It may end up with false positives. > drivers/usb/gadget/udc/goku_udc.h:#define INT_EPnNAK(n) (0x00100 < (n)) > /* 0 < n < 4 */ This is indeed harder, because one has to look at the usage site. julia
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Tue, 6 Feb 2018, Wolfram Sang wrote: > Hi Julia, > > > and got the results below. I can make a version for the kernel shortly. > > It should probably take care of right-shifting, too? I did that too but got no results. Perhaps right shifting constants is pretty uncommon. I can put that in the complete rule though. julia
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Tue, 6 Feb 2018, Dan Carpenter wrote: > On Tue, Feb 06, 2018 at 02:15:51PM +0100, Julia Lawall wrote: > > > > > > On Tue, 6 Feb 2018, Dan Carpenter wrote: > > > > > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote: > > > > In one Renesas driver, I found a typo which turned an intended bit > > > > shift ('<<') > > > > into a comparison ('<'). Because this is a subtle issue, I looked tree > > > > wide for > > > > similar patterns. This small patch series is the outcome. > > > > > > > > Buildbot and checkpatch are happy. Only compile-tested. To be applied > > > > individually per sub-system, I think. I'd think only the net: amd: > > > > patch needs > > > > to be conisdered for stable, but I leave this to people who actually > > > > know this > > > > driver. > > > > > > > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my > > > > setup, only > > > > cppcheck reported a 'coding style' issue with a low prio. > > > > > > > > > > Most of these are inside macros so it makes it complicated for Smatch > > > to warn about them. It might be easier in Coccinelle. Julia the bugs > > > look like this: > > > > > > - reissue_mask |= 0x < 4; > > > + reissue_mask |= 0x << 4; > > > > Thanks. I'll take a look. Do you have an example of the macro issue > > handy? > > > > It's the same: > > #define EXYNOS_CIIMGEFF_PAT_CBCR_MASK ((0xff < 13) | (0xff < 0)) > > Smatch only sees the outside of the macro (where it is used in the code) > and the pre-processed code. I wrote the following rule: @@ constant int x,y; identifier i; type T; @@ ( i < x | x < i | (T)i < x | x < (T)i | * x < y ) and got the results below. I can make a version for the kernel shortly. julia diff -u -p /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h --- /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h +++ /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h @@ -569,7 +569,6 @@ #define EXYNOS_CIIMGEFF_FIN_EMBOSSING (4 << 26) #define EXYNOS_CIIMGEFF_FIN_SILHOUETTE (5 << 26) #define EXYNOS_CIIMGEFF_FIN_MASK (7 << 26) -#define EXYNOS_CIIMGEFF_PAT_CBCR_MASK ((0xff < 13) | (0xff < 0)) /* Real input DMA size register */ #define EXYNOS_CIREAL_ISIZE_AUTOLOAD_ENABLE(1 << 31) diff -u -p /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c --- /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c +++ /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c @@ -595,7 +595,6 @@ isr_done: reissue_mask = 1 << 0; if (!pdata->per_channel_irq) - reissue_mask |= 0x < 4; XP_IOWRITE(pdata, XP_INT_REISSUE_EN, reissue_mask); } diff -u -p /run/shm/linux-next/drivers/video/fbdev/mxsfb.c /tmp/nothing/drivers/video/fbdev/mxsfb.c --- /run/shm/linux-next/drivers/video/fbdev/mxsfb.c +++ /tmp/nothing/drivers/video/fbdev/mxsfb.c @@ -133,8 +133,6 @@ #define VDCTRL4_SYNC_SIGNALS_ON(1 << 18) #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3) -#define DEBUG0_HSYNC (1 < 26) -#define DEBUG0_VSYNC (1 < 25) #define MIN_XRES 120 #define MIN_YRES 120 diff -u -p /run/shm/linux-next/include/drm/drm_scdc_helper.h /tmp/nothing/include/drm/drm_scdc_helper.h --- /run/shm/linux-next/include/drm/drm_scdc_helper.h +++ /tmp/nothing/include/drm/drm_scdc_helper.h @@ -50,9 +50,6 @@ #define SCDC_READ_REQUEST_ENABLE (1 << 0) #define SCDC_STATUS_FLAGS_0 0x40 -#define SCDC_CH2_LOCK (1 < 3) -#define SCDC_CH1_LOCK (1 < 2) -#define SCDC_CH0_LOCK (1 < 1) #define SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK) #define SCDC_CLOCK_DETECT (1 << 0) diff -u -p /run/shm/linux-next/arch/um/drivers/vector_user.h /tmp/nothing/arch/um/drivers/vector_user.h --- /run/shm/linux-next/arch/um/drivers/vector_user.h +++ /tmp/nothing/arch/um/drivers/vector_user.h @@ -61,8 +61,6 @@ struct vector_fds { }; #define VECTOR_READ1 -#define VECTOR_WRITE (1 < 1) -#define VECTOR_HEADERS (1 < 2) extern struct arglist *uml_parse_vector_ifspec(char *arg); diff -u -p /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h --- /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Tue, 6 Feb 2018, Dan Carpenter wrote: > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote: > > In one Renesas driver, I found a typo which turned an intended bit shift > > ('<<') > > into a comparison ('<'). Because this is a subtle issue, I looked tree wide > > for > > similar patterns. This small patch series is the outcome. > > > > Buildbot and checkpatch are happy. Only compile-tested. To be applied > > individually per sub-system, I think. I'd think only the net: amd: patch > > needs > > to be conisdered for stable, but I leave this to people who actually know > > this > > driver. > > > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup, > > only > > cppcheck reported a 'coding style' issue with a low prio. > > > > Most of these are inside macros so it makes it complicated for Smatch > to warn about them. It might be easier in Coccinelle. Julia the bugs > look like this: > > - reissue_mask |= 0x < 4; > + reissue_mask |= 0x << 4; Thanks. I'll take a look. Do you have an example of the macro issue handy? julia > > regards, > dan carpenter > > > Wolfram Sang (4): > > v4l: vsp1: fix mask creation for MULT_ALPHA_RATIO > > drm/exynos: fix comparison to bitshift when dealing with a mask > > v4l: dvb-frontends: stb0899: fix comparison to bitshift when dealing > > with a mask > > net: amd-xgbe: fix comparison to bitshift when dealing with a mask > > > > drivers/gpu/drm/exynos/regs-fimc.h| 2 +- > > drivers/media/dvb-frontends/stb0899_reg.h | 8 > > drivers/media/platform/vsp1/vsp1_regs.h | 2 +- > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > 4 files changed, 7 insertions(+), 7 deletions(-) > > > > -- > > 2.11.0 >
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bart Van Assche wrote: > On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote: > > On Tue, 2 Jan 2018, Bob Peterson wrote: > > > - Original Message - > > > > - Original Message - > > > > > > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > > > and I don't like the thought of re-combining them all. > > > > Actually, the point of the patch was to remove the unnecessary \n at the > > end of the string, because log_print will add another one. If you prefer > > to keep the string broken up, I can resend the patch in that form, but > > without the unnecessary \n. > > Please combine any user-visible strings into a single line for which the > unneeded newline is dropped since these strings are modified anyway by > your patch. That is what the submitted patch (2/12 specifically) did. julia
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | - Original Message - > | | Drop newline at the end of a message string when the printing function > adds > | | a newline. > | > | Hi Julia, > | > | NACK. > | > | As much as it's a pain when searching the source code for output strings, > | this patch set goes against the accepted Linux coding style document. See: > | > | > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings > | > | Regards, > | > | Bob Peterson > | > | > Hm. I guess I stand corrected. The document reads: > > "However, never break user-visible strings such as printk messages, because > that breaks the ability to grep for them." > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > and I don't like the thought of re-combining them all. Actually, the point of the patch was to remove the unnecessary \n at the end of the string, because log_print will add another one. If you prefer to keep the string broken up, I can resend the patch in that form, but without the unnecessary \n. julia
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | Drop newline at the end of a message string when the printing function adds > | a newline. > > Hi Julia, > > NACK. > > As much as it's a pain when searching the source code for output strings, > this patch set goes against the accepted Linux coding style document. See: > > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings I don't think that's the case: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." julia > > Regards, > > Bob Peterson > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers
On Tue, 2 Jan 2018, Dan Carpenter wrote: > On Wed, Dec 20, 2017 at 11:30:01AM +0100, Julia Lawall wrote: > > > > > > On Wed, 20 Dec 2017, Dan Carpenter wrote: > > > > > On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: > > > > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client) > > > > dev_err(>dev, "gpio request/direction_output > > > > fail"); > > > > goto fail2; > > > > } > > > > - if (ACPI_HANDLE(>dev)) > > > > - err = atomisp_register_i2c_module(>sd, NULL, > > > > LED_FLASH); > > > > - return 0; > > > > + return atomisp_register_i2c_module(>sd, NULL, LED_FLASH); > > > > fail2: > > > > media_entity_cleanup(>sd.entity); > > > > v4l2_ctrl_handler_free(>ctrl_handler); > > > > > > Actually every place where we directly return a function call is wrong > > > and needs error handling added. I've been meaning to write a Smatch > > > check for this because it's a common anti-pattern we don't check the > > > last function call for errors. > > > > > > Someone could probably do the same in Coccinelle if they want. > > > > I'm not sure what you are suggesting. Is every case of return f(...); > > for any f wrong? Or is it a particular function that is of concern? Or > > would it be that every function call that has error handling somewhere > > should have error handling everywhere? Or is it related to what seems to > > be the problem in the above code that err is initialized but nothing > > happens to it? > > > > I was just thinking that it's a common pattern to treat the last > function call differently and one mistake I often see looks like this: > > ret = frob(); > if (ret) { > cleanup(); > return ret; > } > > return another_function(); > > No error handling for the last function call. OK, I see. When there was error handling code along the way, a direct return of a function that could fail needs error handling code too. Thanks for the clarification, julia
[PATCH 00/12] drop unneeded newline
Drop newline at the end of a message string when the printing function adds a newline. The complete semantic patch that detects this issue is as shown below (http://coccinelle.lip6.fr/). It works in two phases - the first phase counts how many uses of a function involve a newline and how many don't, and then the second phase removes newlines in the case of calls where a newline is used one fourth of the times or less. This approach is only moderately reliable, and all patches have been checked to ensure that the newline is not needed. This also converts some cases of string concatenation to single strings in modified code, as this improves greppability. // virtual after_start @initialize:ocaml@ @@ let withnl = Hashtbl.create 101 let withoutnl = Hashtbl.create 101 let ignore = ["strcpy";"strlcpy";"strcat";"strlcat";"strcmp";"strncmp";"strcspn"; "strsep";"sprintf";"printf";"strncasecmp";"seq_printf";"strstr";"strspn"; "strlen";"strpbrk";"strtok_r";"memcmp";"memcpy"] let dignore = ["tools";"samples"] let inc tbl k = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref 0 in Hashtbl.add tbl k cell; cell in cell := 1 + !cell let endnl c = let len = String.length c in try String.get c (len-3) = '\\' && String.get c (len-2) = 'n' && String.get c (len-1) = '"' with _ -> false let clean_string s extra = let pieces = Str.split (Str.regexp "\" \"") s in let nonempty s = not (s = "") && String.get s 0 = '"' && not (String.get s 1 = '"') in let rec loop = function [] -> [] | [x] -> [x] | x::y::rest -> if nonempty x && nonempty y then let xend = String.get x (String.length x - 2) = ' ' in let yend = String.get y 1 = ' ' in match (xend,yend) with (true,false) | (false,true) -> x :: (loop (y::rest)) | (true,true) -> x :: (loop (((String.sub y 0 (String.length y - 2))^"\"")::rest)) | (false,false) -> ((String.sub x 0 (String.length x - 1)) ^ " \"") :: (loop (y::rest)) else x :: (loop (y::rest)) in (String.concat "" (loop pieces))^extra @r depends on !after_start@ constant char[] c; expression list[n] es; identifier f; position p; @@ f@p(es,c,...) @script:ocaml@ f << r.f; n << r.n; p << r.p; c << r.c; @@ let pieces = Str.split (Str.regexp "/") (List.hd p).file in if not (List.mem f ignore) && List.for_all (fun x -> not (List.mem x pieces)) dignore then (if endnl c then inc withnl (f,n) else inc withoutnl (f,n)) @finalize:ocaml depends on !after_start@ w1 << merge.withnl; w2 << merge.withoutnl; @@ let names = ref [] in let incn tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> begin let cell = ref 0 in Hashtbl.add tbl k cell; cell end in (if not (List.mem k !names) then names := k :: !names); cell := !v + !cell in List.iter (function w -> Hashtbl.iter (incn withnl) w) w1; List.iter (function w -> Hashtbl.iter (incn withoutnl) w) w2; List.iter (function name -> let wth = try !(Hashtbl.find withnl name) with _ -> 0 in let wo = try !(Hashtbl.find withoutnl name) with _ -> 0 in if wth > 0 && wth <= wo / 3 then Hashtbl.remove withnl name else (Printf.eprintf "dropping %s %d %d\n" (fst name) wth wo; Hashtbl.remove withoutnl name; Hashtbl.remove withnl name)) !names; let it = new iteration() in it#add_virtual_rule After_start; it#register() @s1 depends on after_start@ constant char[] c; expression list[n] es; identifier f; position p; @@ f(es,c@p,...) @script:ocaml s2@ f << s1.f; n << s1.n; c << s1.c; newc; @@ try let _ = Hashtbl.find withnl (f,n) in if endnl c then Coccilib.include_match false else newc := make_expr(clean_string (String.sub c 0 (String.length c - 1)) "\\n\"") with Not_found -> try let _ = Hashtbl.find withoutnl (f,n) in if endnl c then newc := make_expr(clean_string (String.sub c 0 (String.length c - 3)) "\"") else Coccilib.include_match false with Not_found -> Coccilib.include_match false @@ constant char[] s1.c; position s1.p; expression s2.newc; @@ - c@p + newc // --- arch/arm/mach-davinci/board-da850-evm.c |4 ++-- drivers/block/DAC960.c |4 ++-- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c| 12 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |2 +- drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c |2 +- drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |2 +- drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c |2 +- drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++- drivers/s390/block/dasd_diag.c |3 +-- drivers/scsi/hpsa.c |2 +- fs/dlm/plock.c |3 +-- fs/ext2/super.c
[PATCH 09/12] [media] pvrusb2: drop unneeded newline
pvr2_trace prints a newline at the end of the message string, so the message string does not need to include a newline explicitly. Done using Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c index 09bd6c6..e035316 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c @@ -2351,7 +2351,8 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf, if (hdw_desc == NULL) { pvr2_trace(PVR2_TRACE_INIT, "pvr2_hdw_create: No device description pointer, unable to continue."); - pvr2_trace(PVR2_TRACE_INIT, "If you have a new device type, please contact Mike Isely <is...@pobox.com> to get it included in the driver\n"); + pvr2_trace(PVR2_TRACE_INIT, + "If you have a new device type, please contact Mike Isely <is...@pobox.com> to get it included in the driver"); goto fail; }
Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers
On Wed, 20 Dec 2017, Dan Carpenter wrote: > On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: > > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client) > > dev_err(>dev, "gpio request/direction_output fail"); > > goto fail2; > > } > > - if (ACPI_HANDLE(>dev)) > > - err = atomisp_register_i2c_module(>sd, NULL, LED_FLASH); > > - return 0; > > + return atomisp_register_i2c_module(>sd, NULL, LED_FLASH); > > fail2: > > media_entity_cleanup(>sd.entity); > > v4l2_ctrl_handler_free(>ctrl_handler); > > Actually every place where we directly return a function call is wrong > and needs error handling added. I've been meaning to write a Smatch > check for this because it's a common anti-pattern we don't check the > last function call for errors. > > Someone could probably do the same in Coccinelle if they want. I'm not sure what you are suggesting. Is every case of return f(...); for any f wrong? Or is it a particular function that is of concern? Or would it be that every function call that has error handling somewhere should have error handling everywhere? Or is it related to what seems to be the problem in the above code that err is initialized but nothing happens to it? julia
Re: Adjustments for a lot of function implementations
On Mon, 30 Oct 2017, SF Markus Elfring wrote: > > While we do not mind cleanup patches, the way you post them (one fix per > > file) > > I find it safer in this way while I was browsing through the landscape of > Linux > software components. > > > > is really annoying and takes us too much time to review. > > It is just the case that there are so many remaining open issues. > > > > I'll take the "Fix a possible null pointer" patch since it is an actual bug > > fix, > > Thanks for a bit of change acceptance. > > > > but will reject the others, not just this driver but all of them that are > > currently > > pending in our patchwork (https://patchwork.linuxtv.org). > > Will any chances evolve to integrate 146 patches in any other combination? > > > > Feel free to repost, but only if you organize the patch as either fixing > > the same type of > > issue for a whole subdirectory (media/usb, media/pci, etc) Just for the record, while this may work for media, it won't work for all subsystems. One will quickly get a complaint that the big patch needs to go into multiple trees. julia > > Can we achieve an agreement on the shown change patterns? > > Is a consensus possible for involved update candidates? > > > > or fixing all issues for a single driver. > > I find that I did this already. > > > > Actual bug fixes (like the null pointer patch in this series) can still be > > posted as > > separate patches, but cleanups shouldn't. > > I got an other software development opinion. > > > > Just so you know, I'll reject any future patch series that do not follow > > these rules. > > Just use common sense when posting these things in the future. > > Do we need to try any additional communication tools out? > > > > I would also suggest that your time might be spent more productively if you > > would > > work on some more useful projects. > > I hope that various change possibilities (from my selection) will become > useful > for more Linux users. > How will the clarification evolve further? > > > Regards, > Markus > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [PATCH] [media] bdisp: remove redundant assignment to pix
On Sun, 29 Oct 2017, Colin King wrote: > From: Colin Ian King <colin.k...@canonical.com> > > Pointer pix is being initialized to a value and a little later > being assigned the same value again. Remove the redundant second > duplicate assignment. Cleans up the clang warning: > > drivers/media/platform/sti/bdisp/bdisp-v4l2.c:726:26: warning: Value > stored to 'pix' during its initialization is never read > > Signed-off-by: Colin Ian King <colin.k...@canonical.com> > --- > drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > index 939da6da7644..14e99aeae140 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > @@ -731,7 +731,6 @@ static int bdisp_g_fmt(struct file *file, void *fh, > struct v4l2_format *f) > return PTR_ERR(frame); > } > > - pix = >fmt.pix; Why not keep this one and drop the first one? Maybe it would be nice to keep all the initializations related to pix together? julia > pix->width = frame->width; > pix->height = frame->height; > pix->pixelformat = frame->fmt->pixelformat; > -- > 2.14.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [Outreachy kernel] [PATCH v2 1/2] staging: atomisp2: cleanup null check on memory allocation
On Sat, 14 Oct 2017, Aishwarya Pant wrote: > For memory allocation functions that fail with a NULL return value, it > is preferred to use the (!x) test in place of (x == NULL). > > Changes in atomisp2/css2400/sh_css.c were done by hand. > > Done with the help of the following cocci script: > > @@ > type T; > T* p; > statement s,s1; > @@ > > p = > \(devm_kzalloc\|devm_ioremap\|usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\| > > kmalloc\|kmalloc_array\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\| >kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\|devm_kzalloc\)(...) > ...when != p > > if ( > - p == NULL > + !p > ) s > else s1 > > Signed-off-by: Aishwarya Pant <aishp...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > > -- > Changes in atomisp2/css2400/sh_css.c were done by hand, the above script > was not able to match the pattern if (a->b != null). > > v2 changes: > None, just rebase and re-send > --- > .../media/atomisp/pci/atomisp2/css2400/sh_css.c| 36 > +++--- > .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 6 ++-- > .../pci/atomisp2/css2400/sh_css_param_shading.c| 2 +- > 3 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > index e882b5596813..56de641d8848 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > @@ -5607,13 +5607,13 @@ static enum ia_css_err load_video_binaries(struct > ia_css_pipe *pipe) > mycs->num_yuv_scaler = cas_scaler_descr.num_stage; > mycs->yuv_scaler_binary = kzalloc(cas_scaler_descr.num_stage * > sizeof(struct ia_css_binary), GFP_KERNEL); > - if (mycs->yuv_scaler_binary == NULL) { > + if (!mycs->yuv_scaler_binary) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > return err; > } > mycs->is_output_stage = kzalloc(cas_scaler_descr.num_stage > * sizeof(bool), GFP_KERNEL); > - if (mycs->is_output_stage == NULL) { > + if (!mycs->is_output_stage) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > return err; > } > @@ -6258,14 +6258,14 @@ static enum ia_css_err load_primary_binaries( > mycs->num_yuv_scaler = cas_scaler_descr.num_stage; > mycs->yuv_scaler_binary = kzalloc(cas_scaler_descr.num_stage * > sizeof(struct ia_css_binary), GFP_KERNEL); > - if (mycs->yuv_scaler_binary == NULL) { > + if (!mycs->yuv_scaler_binary) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > IA_CSS_LEAVE_ERR_PRIVATE(err); > return err; > } > mycs->is_output_stage = kzalloc(cas_scaler_descr.num_stage * > sizeof(bool), GFP_KERNEL); > - if (mycs->is_output_stage == NULL) { > + if (!mycs->is_output_stage) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > IA_CSS_LEAVE_ERR_PRIVATE(err); > return err; > @@ -6982,27 +6982,27 @@ static enum ia_css_err > ia_css_pipe_create_cas_scaler_desc_single_output( > } > > descr->in_info = kmalloc(descr->num_stage * sizeof(struct > ia_css_frame_info), GFP_KERNEL); > - if (descr->in_info == NULL) { > + if (!descr->in_info) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > goto ERR; > } > descr->internal_out_info = kmalloc(descr->num_stage * sizeof(struct > ia_css_frame_info), GFP_KERNEL); > - if (descr->internal_out_info == NULL) { > + if (!descr->internal_out_info) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > goto ERR; > } > descr->out_info = kmalloc(descr->num_stage * sizeof(struct > ia_css_frame_info), GFP_KERNEL); > - if (descr->out_info == NULL) { > + if (!descr->out_info) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; > goto ERR; > } > descr->vf_info = kmalloc(descr->num_stage * sizeof(struct > ia_css_frame_info), GFP_KERNEL); > - if (descr->vf_info == NULL) { > + if (!descr->vf_info) { > err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY; >
Re: [Outreachy kernel] [PATCH] Staging: media: atomisp: pci: Eliminate use of typedefs for struct
On Sat, 7 Oct 2017, Srishti Sharma wrote: > The use of typedefs for struct is discouraged, and hence can be > eliminated. Done using the following semantic patch by coccinelle. > > @r1@ > type T; > @@ > > typedef struct {...} T; > > @script: python p@ > T << r1.T; > T1; > @@ > > if T[-2:] == "_t" or T[-2:] == "_T": > coccinelle.T1 = T[:-2] > else: > coccinelle.T1 = T > > print T, T1 > @r2@ > type r1.T; > identifier p.T1; > @@ > > - typedef > struct > + T1 > { > ... > } > - T > ; > > @r3@ > type r1.T; > identifier p.T1; > @@ > > - T > + struct T1 > > Signed-off-by: Srishti Sharma <srishtis...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > --- > .../media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c | 6 > +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c > > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c > index d9178e8..6d9bceb 100644 > --- > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c > +++ > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c > @@ -37,7 +37,7 @@ more details. > #include "ia_css_spctrl.h" > #include "ia_css_debug.h" > > -typedef struct { > +struct spctrl_context_info { > struct ia_css_sp_init_dmem_cfg dmem_config; > uint32_tspctrl_config_dmem_addr; /** location of dmem_cfg in > SP dmem */ > uint32_tspctrl_state_dmem_addr; > @@ -45,9 +45,9 @@ typedef struct { > hrt_vaddresscode_addr; /* sp firmware location in host > mem-DDR*/ > uint32_tcode_size; > char *program_name; /* used in case of PLATFORM_SIM */ > -} spctrl_context_info; > +}; > > -static spctrl_context_info spctrl_cofig_info[N_SP_ID]; > +static struct spctrl_context_info spctrl_cofig_info[N_SP_ID]; > static bool spctrl_loaded[N_SP_ID] = {0}; > > /* Load firmware */ > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/1507384322-16584-1-git-send-email-srishtishar%40gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [PATCH] [media] ov5645: I2C address change (fwd)
Hello, It seems that an unlock is missing on line 764. julia -- Forwarded message -- Date: Wed, 4 Oct 2017 05:59:09 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH] [media] ov5645: I2C address change CC: kbuild-...@01.org In-Reply-To: <1506950925-13924-1-git-send-email-todor.to...@linaro.org> TO: Todor Tomov <todor.to...@linaro.org> CC: mche...@kernel.org, sakari.ai...@linux.intel.com, hansv...@cisco.com, linux-media@vger.kernel.org, linux-ker...@vger.kernel.org, Todor Tomov <todor.to...@linaro.org> CC: linux-media@vger.kernel.org, linux-ker...@vger.kernel.org, Todor Tomov <todor.to...@linaro.org> Hi Todor, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.14-rc3 next-20170929] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Todor-Tomov/ov5645-I2C-address-change/20171003-234231 base: git://linuxtv.org/media_tree.git master :: branch date: 6 hours ago :: commit date: 6 hours ago >> drivers/media/i2c/ov5645.c:806:1-7: preceding lock on line 760 # https://github.com/0day-ci/linux/commit/c222075023642217170e2ef95f48efef079f9bcd git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout c222075023642217170e2ef95f48efef079f9bcd vim +806 drivers/media/i2c/ov5645.c 9cae9722 Todor Tomov 2017-04-11 747 9cae9722 Todor Tomov 2017-04-11 748 static int ov5645_s_power(struct v4l2_subdev *sd, int on) 9cae9722 Todor Tomov 2017-04-11 749 { 9cae9722 Todor Tomov 2017-04-11 750struct ov5645 *ov5645 = to_ov5645(sd); 9cae9722 Todor Tomov 2017-04-11 751int ret = 0; 9cae9722 Todor Tomov 2017-04-11 752 9cae9722 Todor Tomov 2017-04-11 753mutex_lock(>power_lock); 9cae9722 Todor Tomov 2017-04-11 754 9cae9722 Todor Tomov 2017-04-11 755/* If the power count is modified from 0 to != 0 or from != 0 to 0, 9cae9722 Todor Tomov 2017-04-11 756 * update the power state. 9cae9722 Todor Tomov 2017-04-11 757 */ 9cae9722 Todor Tomov 2017-04-11 758if (ov5645->power_count == !on) { 9cae9722 Todor Tomov 2017-04-11 759if (on) { c2220750 Todor Tomov 2017-10-02 @760 mutex_lock(_lock); c2220750 Todor Tomov 2017-10-02 761 9cae9722 Todor Tomov 2017-04-11 762ret = ov5645_set_power_on(ov5645); 9cae9722 Todor Tomov 2017-04-11 763if (ret < 0) 9cae9722 Todor Tomov 2017-04-11 764goto exit; 9cae9722 Todor Tomov 2017-04-11 765 c2220750 Todor Tomov 2017-10-02 766ret = ov5645_write_reg_to(ov5645, 0x3100, c2220750 Todor Tomov 2017-10-02 767 ov5645->i2c_client->addr, 0x78); c2220750 Todor Tomov 2017-10-02 768if (ret < 0) { c2220750 Todor Tomov 2017-10-02 769 dev_err(ov5645->dev, c2220750 Todor Tomov 2017-10-02 770"could not change i2c address\n"); c2220750 Todor Tomov 2017-10-02 771 ov5645_set_power_off(ov5645); c2220750 Todor Tomov 2017-10-02 772 mutex_unlock(_lock); c2220750 Todor Tomov 2017-10-02 773goto exit; c2220750 Todor Tomov 2017-10-02 774} c2220750 Todor Tomov 2017-10-02 775 c2220750 Todor Tomov 2017-10-02 776 mutex_unlock(_lock); c2220750 Todor Tomov 2017-10-02 777 9cae9722 Todor Tomov 2017-04-11 778ret = ov5645_set_register_array(ov5645, 9cae9722 Todor Tomov 2017-04-11 779 ov5645_global_init_setting, 9cae9722 Todor Tomov 2017-04-11 780 ARRAY_SIZE(ov5645_global_init_setting)); 9cae9722 Todor Tomov 2017-04-11 781if (ret < 0) { 9cae9722 Todor Tomov 2017-04-11 782 dev_err(ov5645->dev, 9cae9722 Todor Tomov 2017-04-11 783"could not set init registers\n"); 9cae9722 Todor Tomov 2017-04-11 784 ov5645_set_power_off(ov5645); 9cae9722 Todor Tomov 2017-04-11 785goto exit; 9cae9722 Todor Tomov 2017-04-11 786} 9cae9722 Todor Tomov 2017-04-11 787 9cae9722 Todor Tomov 2017-04-11 788ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, 9cae9722 Todor Tomov 2017-04-11 789 OV5645_SYSTEM_CTRL0_STOP); 9cae9722 Todor Tomov 2017-04-11 790if (ret < 0) { 9cae9722 Todor Tomov 2017-04-11 791 ov5645_set_power_off(ov5645); 9cae9722 Todor Tomov 2017-04-11 792goto exi
Re: [media] spca500: Use common error handling code in spca500_synch310()
On Fri, 22 Sep 2017, SF Markus Elfring wrote: > >>return 0; > >> -error: > >> + > >> +report_failure: > >> + PERR("Set packet size: set interface error"); > >>return -EBUSY; > >> } > > > > Why change the label name? > > I find the suggested variant a bi better. > > > > They are both equally uninformative. > > Which identifier would you find appropriate there? error was fine. julia
Re: [PATCH] [media] spca500: Use common error handling code in spca500_synch310()
On Fri, 22 Sep 2017, SF Markus Elfring wrote: > From: Markus Elfring <elfr...@users.sourceforge.net> > Date: Fri, 22 Sep 2017 18:45:07 +0200 > > Adjust a jump target so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> > --- > drivers/media/usb/gspca/spca500.c | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/usb/gspca/spca500.c > b/drivers/media/usb/gspca/spca500.c > index da2d9027914c..1f224f5e5b19 100644 > --- a/drivers/media/usb/gspca/spca500.c > +++ b/drivers/media/usb/gspca/spca500.c > @@ -501,7 +501,6 @@ static int spca500_full_reset(struct gspca_dev *gspca_dev) > static int spca500_synch310(struct gspca_dev *gspca_dev) > { > - if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0) { > - PERR("Set packet size: set interface error"); > - goto error; > - } > + if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0) > + goto report_failure; > + > spca500_ping310(gspca_dev); > @@ -514,12 +513,12 @@ static int spca500_synch310(struct gspca_dev *gspca_dev) > /* Windoze use pipe with altsetting 6 why 7 here */ > - if (usb_set_interface(gspca_dev->dev, > - gspca_dev->iface, > - gspca_dev->alt) < 0) { > - PERR("Set packet size: set interface error"); > - goto error; > - } > + if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, gspca_dev->alt) > + < 0) > + goto report_failure; > + > return 0; > -error: > + > +report_failure: > + PERR("Set packet size: set interface error"); > return -EBUSY; > } Why change the label name? They are both equally uninformative. julia
Re: [Outreachy kernel] [PATCH] Staging: media: atomisp: Merge assignment with return
On Tue, 12 Sep 2017, Srishti Sharma wrote: > Merge the assignment and the return statements to return the value > directly. Done using the following semantic patch by coccinelle. > > @@ > local idexpression ret; > expression e; > @@ > > -ret = > +return > e; > -return ret; > > Signed-off-by: Srishti Sharma <srishtis...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > --- > drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c > b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c > index 11162f5..e6ddfbf 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c > +++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c > @@ -1168,13 +1168,9 @@ void hmm_bo_free_pages(struct hmm_buffer_object *bo) > > int hmm_bo_page_allocated(struct hmm_buffer_object *bo) > { > - int ret; > - > check_bo_null_return(bo, 0); > > - ret = bo->status & HMM_BO_PAGE_ALLOCED; > - > - return ret; > + return bo->status & HMM_BO_PAGE_ALLOCED; > } > > /* > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/1505226307-5119-1-git-send-email-srishtishar%40gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [Outreachy kernel] [PATCH] Staging: media: atomisp: Merge assignment with return
On Tue, 12 Sep 2017, Srishti Sharma wrote: > Merge the assignment and the return statements to return the value > directly. Done using the following semantic patch by coccinelle. > > @@ > local idexpression ret; > expression e; > @@ > > -ret = > +return > e; > -return ret; > > Signed-off-by: Srishti Sharma <srishtis...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > --- > drivers/staging/media/atomisp/i2c/ov5693/ov5693.c | 11 ++- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c > b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c > index 1236425..2195011 100644 > --- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c > +++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c > @@ -945,12 +945,8 @@ static int ad5823_t_focus_vcm(struct v4l2_subdev *sd, > u16 val) > > int ad5823_t_focus_abs(struct v4l2_subdev *sd, s32 value) > { > - int ret; > - > value = min(value, AD5823_MAX_FOCUS_POS); > - ret = ad5823_t_focus_vcm(sd, value); > - > - return ret; > + return ad5823_t_focus_vcm(sd, value); > } > > static int ov5693_t_focus_abs(struct v4l2_subdev *sd, s32 value) > @@ -1332,7 +1328,6 @@ static int power_ctrl(struct v4l2_subdev *sd, bool flag) > > static int gpio_ctrl(struct v4l2_subdev *sd, bool flag) > { > - int ret; > struct ov5693_device *dev = to_ov5693_sensor(sd); > > if (!dev || !dev->platform_data) > @@ -1342,9 +1337,7 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag) > if (dev->platform_data->gpio_ctrl) > return dev->platform_data->gpio_ctrl(sd, flag); > > - ret = dev->platform_data->gpio0_ctrl(sd, flag); > - > - return ret; > + return dev->platform_data->gpio0_ctrl(sd, flag); > } > > static int __power_up(struct v4l2_subdev *sd) > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/1505225549-4432-1-git-send-email-srishtishar%40gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [Outreachy kernel] [PATCH] Staging: media: omap4iss: Use WARN_ON() instead of BUG_ON().
On Fri, 8 Sep 2017, Srishti Sharma wrote: > Use WARN_ON() instead of BUG_ON() to avoid crashing the kernel. > > Signed-off-by: Srishti Sharma <srishtis...@gmail.com> > --- > drivers/staging/media/omap4iss/iss.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/omap4iss/iss.c > b/drivers/staging/media/omap4iss/iss.c > index c26c99fd..b1036ba 100644 > --- a/drivers/staging/media/omap4iss/iss.c > +++ b/drivers/staging/media/omap4iss/iss.c > @@ -893,7 +893,7 @@ void omap4iss_put(struct iss_device *iss) > return; > > mutex_lock(>iss_mutex); > - BUG_ON(iss->ref_count == 0); > + WARN_ON(iss->ref_count == 0); > if (--iss->ref_count == 0) { Won't this then infinite loop? julia > iss_disable_interrupts(iss); > /* Reset the ISS if an entity has failed to stop. This is the > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/1504879698-5855-1-git-send-email-srishtishar%40gmail.com. > For more options, visit https://groups.google.com/d/optout. >
[PATCH v2] media: ddbridge: constify stv0910_p and lnbh25_cfg
These structures are only copied into other structures, so they can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- v2: fix typo in the commit message drivers/media/pci/ddbridge/ddbridge-core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 7e164a37..7ff570b 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -800,14 +800,14 @@ static int tuner_attach_stv6110(struct ddb_input *input, int type) return 0; } -static struct stv0910_cfg stv0910_p = { +static const struct stv0910_cfg stv0910_p = { .adr = 0x68, .parallel = 1, .rptlvl = 4, .clk = 3000, }; -static struct lnbh25_config lnbh25_cfg = { +static const struct lnbh25_config lnbh25_cfg = { .i2c_address = 0x0c << 1, .data2_config = LNBH25_TEN };
[PATCH] media: ddbridge: constify stv0910_p and lnbh25_cfg
These structures are only copied into other stuructures, so they can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/pci/ddbridge/ddbridge-core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 7e164a37..7ff570b 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -800,14 +800,14 @@ static int tuner_attach_stv6110(struct ddb_input *input, int type) return 0; } -static struct stv0910_cfg stv0910_p = { +static const struct stv0910_cfg stv0910_p = { .adr = 0x68, .parallel = 1, .rptlvl = 4, .clk = 3000, }; -static struct lnbh25_config lnbh25_cfg = { +static const struct lnbh25_config lnbh25_cfg = { .i2c_address = 0x0c << 1, .data2_config = LNBH25_TEN };
[PATCH] [media] pxa_camera: constify v4l2_clk_ops structure
This v4l2_clk_ops structure is only passed as the first argument of v4l2_clk_register, which is const, so the v4l2_clk_ops structure can also be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/pxa_camera.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c index 0d4af6d..4a800a4 100644 --- a/drivers/media/platform/pxa_camera.c +++ b/drivers/media/platform/pxa_camera.c @@ -2100,7 +2100,7 @@ static int pxac_fops_camera_release(struct file *filp) .vidioc_unsubscribe_event = v4l2_event_unsubscribe, }; -static struct v4l2_clk_ops pxa_camera_mclk_ops = { +static const struct v4l2_clk_ops pxa_camera_mclk_ops = { }; static const struct video_device pxa_camera_videodev_template = {
[PATCH] [media] v4l2: av7110_v4l: constify v4l2_audio structure
This v4l2_audio structure is only copied into other structures, so it can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/pci/ttpci/av7110_v4l.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/ttpci/av7110_v4l.c b/drivers/media/pci/ttpci/av7110_v4l.c index 397fe14..e4cf42c 100644 --- a/drivers/media/pci/ttpci/av7110_v4l.c +++ b/drivers/media/pci/ttpci/av7110_v4l.c @@ -218,7 +218,7 @@ static int stv0297_set_tv_freq(struct saa7146_dev *dev, u32 freq) static struct saa7146_standard dvb_standard[]; static struct saa7146_standard standard[]; -static struct v4l2_audio msp3400_v4l2_audio = { +static const struct v4l2_audio msp3400_v4l2_audio = { .index = 0, .name = "Television", .capability = V4L2_AUDCAP_STEREO
Re: [PATCH 6/6] [media] uvcvideo: constify video_subdev structures
On Tue, 8 Aug 2017, Laurent Pinchart wrote: > Hi Julia, > > Thank you for the patch. > > On Tuesday 08 Aug 2017 12:58:32 Julia Lawall wrote: > > uvc_subdev_ops is only passed as the second argument of > > v4l2_subdev_init, which is const, so uvc_subdev_ops can be > > const as well. > > > > Done with the help of Coccinelle. > > > > Signed-off-by: Julia Lawall <julia.law...@lip6.fr> > > Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > > and applied to my tree (with the first word of the commit message after the > prefix capitalized to match the rest of the driver's commit messages, let me > know if that's a problem :-)). No, of course not. I will try to watch out for that in the future. julia > > > > --- > > drivers/media/usb/uvc/uvc_entity.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_entity.c > > b/drivers/media/usb/uvc/uvc_entity.c index ac386bb..554063c 100644 > > --- a/drivers/media/usb/uvc/uvc_entity.c > > +++ b/drivers/media/usb/uvc/uvc_entity.c > > @@ -61,7 +61,7 @@ static int uvc_mc_create_links(struct uvc_video_chain > > *chain, return 0; > > } > > > > -static struct v4l2_subdev_ops uvc_subdev_ops = { > > +static const struct v4l2_subdev_ops uvc_subdev_ops = { > > }; > > > > void uvc_mc_cleanup_entity(struct uvc_entity *entity) > > -- > Regards, > > Laurent Pinchart > >
[PATCH 1/6] [media] v4l: mt9t001: constify video_subdev structures
The v4l2_subdev_ops structure is only passed as the third argument of v4l2_i2c_subdev_init, which is const, so the v4l2_subdev_ops structure can be const as well. The other structures are only stored in the v4l2_subdev_ops structure, all the fields of which are const, so these structures can also be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/i2c/mt9t001.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c index 842017f..9d981d9 100644 --- a/drivers/media/i2c/mt9t001.c +++ b/drivers/media/i2c/mt9t001.c @@ -822,15 +822,15 @@ static int mt9t001_close(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) return mt9t001_set_power(subdev, 0); } -static struct v4l2_subdev_core_ops mt9t001_subdev_core_ops = { +static const struct v4l2_subdev_core_ops mt9t001_subdev_core_ops = { .s_power = mt9t001_set_power, }; -static struct v4l2_subdev_video_ops mt9t001_subdev_video_ops = { +static const struct v4l2_subdev_video_ops mt9t001_subdev_video_ops = { .s_stream = mt9t001_s_stream, }; -static struct v4l2_subdev_pad_ops mt9t001_subdev_pad_ops = { +static const struct v4l2_subdev_pad_ops mt9t001_subdev_pad_ops = { .enum_mbus_code = mt9t001_enum_mbus_code, .enum_frame_size = mt9t001_enum_frame_size, .get_fmt = mt9t001_get_format, @@ -839,7 +839,7 @@ static int mt9t001_close(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) .set_selection = mt9t001_set_selection, }; -static struct v4l2_subdev_ops mt9t001_subdev_ops = { +static const struct v4l2_subdev_ops mt9t001_subdev_ops = { .core = _subdev_core_ops, .video = _subdev_video_ops, .pad = _subdev_pad_ops,
[PATCH 6/6] [media] uvcvideo: constify video_subdev structures
uvc_subdev_ops is only passed as the second argument of v4l2_subdev_init, which is const, so uvc_subdev_ops can be const as well. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/uvc/uvc_entity.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c index ac386bb..554063c 100644 --- a/drivers/media/usb/uvc/uvc_entity.c +++ b/drivers/media/usb/uvc/uvc_entity.c @@ -61,7 +61,7 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain, return 0; } -static struct v4l2_subdev_ops uvc_subdev_ops = { +static const struct v4l2_subdev_ops uvc_subdev_ops = { }; void uvc_mc_cleanup_entity(struct uvc_entity *entity)
[PATCH 3/6] staging: atomisp: constify video_subdev structures
These structures are both stored in fields of v4l2_subdev_ops structures, all of which are const, so these structures can be const as well. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/staging/media/atomisp/i2c/ap1302.c |2 +- drivers/staging/media/atomisp/i2c/mt9m114.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/ap1302.c b/drivers/staging/media/atomisp/i2c/ap1302.c index bacffbe..de687c6 100644 --- a/drivers/staging/media/atomisp/i2c/ap1302.c +++ b/drivers/staging/media/atomisp/i2c/ap1302.c @@ -1098,7 +1098,7 @@ static long ap1302_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) }, }; -static struct v4l2_subdev_sensor_ops ap1302_sensor_ops = { +static const struct v4l2_subdev_sensor_ops ap1302_sensor_ops = { .g_skip_frames = ap1302_g_skip_frames, }; diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c b/drivers/staging/media/atomisp/i2c/mt9m114.c index 448e072..36a0636 100644 --- a/drivers/staging/media/atomisp/i2c/mt9m114.c +++ b/drivers/staging/media/atomisp/i2c/mt9m114.c @@ -1806,7 +1806,7 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, u32 *frames) .g_frame_interval = mt9m114_g_frame_interval, }; -static struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = { +static const struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = { .g_skip_frames = mt9m114_g_skip_frames, };
[PATCH 0/6] constify video_subdev structures
The structures of type v4l2_subdev_ops are only passed as the second argument of v4l2_subdev_init or as the third argument of v4l2_i2c_subdev_init, both of which are const. The structures of type v4l2_subdev_core_ops, v4l2_subdev_pad_ops, v4l2_subdev_sensor_ops, v4l2_subdev_video_ops are only stored in fields of v4l2_subdev_ops structures, all of which are const. Thus all of these structures can be declared as const as well. Done with the help of Coccinelle. --- drivers/media/i2c/mt9m111.c |6 +++--- drivers/media/i2c/mt9t001.c |8 drivers/media/platform/exynos4-is/fimc-isp.c |2 +- drivers/media/platform/exynos4-is/fimc-lite.c |2 +- drivers/media/platform/vimc/vimc-debayer.c|2 +- drivers/media/platform/vimc/vimc-scaler.c |2 +- drivers/media/platform/vimc/vimc-sensor.c |2 +- drivers/media/usb/uvc/uvc_entity.c|2 +- drivers/staging/media/atomisp/i2c/ap1302.c|2 +- drivers/staging/media/atomisp/i2c/mt9m114.c |2 +- 10 files changed, 15 insertions(+), 15 deletions(-)
[PATCH 2/6] [media] vimc: constify video_subdev structures
These structures are all only stored in fields of v4l2_subdev_ops structures, all of which are const, so these structures can be const as well. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/vimc/vimc-debayer.c |2 +- drivers/media/platform/vimc/vimc-scaler.c |2 +- drivers/media/platform/vimc/vimc-sensor.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index 033a131..4d663e8 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -373,7 +373,7 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable) return 0; } -static struct v4l2_subdev_video_ops vimc_deb_video_ops = { +static const struct v4l2_subdev_video_ops vimc_deb_video_ops = { .s_stream = vimc_deb_s_stream, }; diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index 0a3e086..e1602e0 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -267,7 +267,7 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int enable) return 0; } -static struct v4l2_subdev_video_ops vimc_sca_video_ops = { +static const struct v4l2_subdev_video_ops vimc_sca_video_ops = { .s_stream = vimc_sca_s_stream, }; diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c index 615c2b1..02e68c8 100644 --- a/drivers/media/platform/vimc/vimc-sensor.c +++ b/drivers/media/platform/vimc/vimc-sensor.c @@ -282,7 +282,7 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable) return 0; } -static struct v4l2_subdev_video_ops vimc_sen_video_ops = { +static const struct v4l2_subdev_video_ops vimc_sen_video_ops = { .s_stream = vimc_sen_s_stream, };
[PATCH 5/6] [media] exynos4-is: constify video_subdev structures
The v4l2_subdev_ops structures are only passed as the second argument of v4l2_subdev_init, which is const, so the v4l2_subdev_ops structures can be const as well. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/exynos4-is/fimc-isp.c |2 +- drivers/media/platform/exynos4-is/fimc-lite.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-isp.c b/drivers/media/platform/exynos4-is/fimc-isp.c index 8efe916..fd793d3 100644 --- a/drivers/media/platform/exynos4-is/fimc-isp.c +++ b/drivers/media/platform/exynos4-is/fimc-isp.c @@ -433,7 +433,7 @@ static void fimc_isp_subdev_unregistered(struct v4l2_subdev *sd) .s_power = fimc_isp_subdev_s_power, }; -static struct v4l2_subdev_ops fimc_is_subdev_ops = { +static const struct v4l2_subdev_ops fimc_is_subdev_ops = { .core = _is_core_ops, .video = _is_subdev_video_ops, .pad = _is_subdev_pad_ops, diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c index 7d3ec5c..30282c5 100644 --- a/drivers/media/platform/exynos4-is/fimc-lite.c +++ b/drivers/media/platform/exynos4-is/fimc-lite.c @@ -1361,7 +1361,7 @@ static void fimc_lite_subdev_unregistered(struct v4l2_subdev *sd) .log_status = fimc_lite_log_status, }; -static struct v4l2_subdev_ops fimc_lite_subdev_ops = { +static const struct v4l2_subdev_ops fimc_lite_subdev_ops = { .core = _lite_core_ops, .video = _lite_subdev_video_ops, .pad = _lite_subdev_pad_ops,
[PATCH 4/6] [media] media: mt9m111: constify video_subdev structures
The v4l2_subdev_ops structure is only passed as the third argument of v4l2_i2c_subdev_init, which is const, so the v4l2_subdev_ops structure can be const as well. The other structures are only stored in the v4l2_subdev_ops structure, all the fields of which are const, so these structures can also be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/i2c/mt9m111.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c index 72e71b7..99b992e 100644 --- a/drivers/media/i2c/mt9m111.c +++ b/drivers/media/i2c/mt9m111.c @@ -835,7 +835,7 @@ static int mt9m111_s_power(struct v4l2_subdev *sd, int on) .s_ctrl = mt9m111_s_ctrl, }; -static struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = { +static const struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = { .s_power= mt9m111_s_power, #ifdef CONFIG_VIDEO_ADV_DEBUG .g_register = mt9m111_g_register, @@ -865,7 +865,7 @@ static int mt9m111_g_mbus_config(struct v4l2_subdev *sd, return 0; } -static struct v4l2_subdev_video_ops mt9m111_subdev_video_ops = { +static const struct v4l2_subdev_video_ops mt9m111_subdev_video_ops = { .g_mbus_config = mt9m111_g_mbus_config, }; @@ -877,7 +877,7 @@ static int mt9m111_g_mbus_config(struct v4l2_subdev *sd, .set_fmt= mt9m111_set_fmt, }; -static struct v4l2_subdev_ops mt9m111_subdev_ops = { +static const struct v4l2_subdev_ops mt9m111_subdev_ops = { .core = _subdev_core_ops, .video = _subdev_video_ops, .pad= _subdev_pad_ops,
[PATCH] [media] vs6624: constify vs6624_default_fmt
The structure vs6624_default_fmt is only copied into another structure field, so it can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/i2c/vs6624.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c index f0741ab..5607382 100644 --- a/drivers/media/i2c/vs6624.c +++ b/drivers/media/i2c/vs6624.c @@ -58,7 +58,7 @@ struct vs6624 { }, }; -static struct v4l2_mbus_framefmt vs6624_default_fmt = { +static const struct v4l2_mbus_framefmt vs6624_default_fmt = { .width = VGA_WIDTH, .height = VGA_HEIGHT, .code = MEDIA_BUS_FMT_UYVY8_2X8,
[PATCH 02/12] [media] media: ti-vpe: vpe: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/ti-vpe/vpe.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c471514..2873c22 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2430,7 +2430,7 @@ static int vpe_release(struct file *file) .vfl_dir= VFL_DIR_M2M, }; -static struct v4l2_m2m_ops m2m_ops = { +static const struct v4l2_m2m_ops m2m_ops = { .device_run = device_run, .job_ready = job_ready, .job_abort = job_abort,
[PATCH 04/12] [media] V4L2: platform: rcar_jpu: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/rcar_jpu.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/rcar_jpu.c b/drivers/media/platform/rcar_jpu.c index d1746ec..070bac3 100644 --- a/drivers/media/platform/rcar_jpu.c +++ b/drivers/media/platform/rcar_jpu.c @@ -1506,7 +1506,7 @@ static void jpu_job_abort(void *priv) jpu_cleanup(ctx, true); } -static struct v4l2_m2m_ops jpu_m2m_ops = { +static const struct v4l2_m2m_ops jpu_m2m_ops = { .device_run = jpu_device_run, .job_ready = jpu_job_ready, .job_abort = jpu_job_abort,
[PATCH 03/12] [media] s5p-g2d: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/s5p-g2d/g2d.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c index 81ed5cd..bd655b5 100644 --- a/drivers/media/platform/s5p-g2d/g2d.c +++ b/drivers/media/platform/s5p-g2d/g2d.c @@ -611,7 +611,7 @@ static irqreturn_t g2d_isr(int irq, void *prv) .vfl_dir= VFL_DIR_M2M, }; -static struct v4l2_m2m_ops g2d_m2m_ops = { +static const struct v4l2_m2m_ops g2d_m2m_ops = { .device_run = device_run, .job_abort = job_abort, };
[PATCH 01/12] [media] st-delta: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/sti/delta/delta-v4l2.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c index ff9850e..b2dc3d2 100644 --- a/drivers/media/platform/sti/delta/delta-v4l2.c +++ b/drivers/media/platform/sti/delta/delta-v4l2.c @@ -1095,7 +1095,7 @@ static int delta_job_ready(void *priv) } /* mem-to-mem ops */ -static struct v4l2_m2m_ops delta_m2m_ops = { +static const struct v4l2_m2m_ops delta_m2m_ops = { .device_run = delta_device_run, .job_ready = delta_job_ready, .job_abort = delta_job_abort,
[PATCH 05/12] [media] vcodec: mediatek: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c index f17a86b..226f908 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c @@ -865,7 +865,7 @@ static void mtk_jpeg_job_abort(void *priv) { } -static struct v4l2_m2m_ops mtk_jpeg_m2m_ops = { +static const struct v4l2_m2m_ops mtk_jpeg_m2m_ops = { .device_run = mtk_jpeg_device_run, .job_ready = mtk_jpeg_job_ready, .job_abort = mtk_jpeg_job_abort,
[PATCH 00/12] constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. --- drivers/media/platform/exynos-gsc/gsc-m2m.c |2 +- drivers/media/platform/exynos4-is/fimc-m2m.c|2 +- drivers/media/platform/m2m-deinterlace.c|2 +- drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c |2 +- drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c|2 +- drivers/media/platform/mx2_emmaprp.c|2 +- drivers/media/platform/rcar_jpu.c |2 +- drivers/media/platform/s5p-g2d/g2d.c|2 +- drivers/media/platform/sti/bdisp/bdisp-v4l2.c |2 +- drivers/media/platform/sti/delta/delta-v4l2.c |2 +- drivers/media/platform/ti-vpe/vpe.c |2 +- drivers/media/platform/vim2m.c |2 +- 12 files changed, 12 insertions(+), 12 deletions(-)
[PATCH 06/12] [media] exynos-gsc: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/exynos-gsc/gsc-m2m.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index 33611a4..2a2994e 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -747,7 +747,7 @@ static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma) .mmap = gsc_m2m_mmap, }; -static struct v4l2_m2m_ops gsc_m2m_ops = { +static const struct v4l2_m2m_ops gsc_m2m_ops = { .device_run = gsc_m2m_device_run, .job_abort = gsc_m2m_job_abort, };
[PATCH 07/12] [media] bdisp: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/sti/bdisp/bdisp-v4l2.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c index 7918b92..939da6d 100644 --- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c +++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c @@ -360,7 +360,7 @@ static void bdisp_device_run(void *priv) bdisp_job_finish(ctx, VB2_BUF_STATE_ERROR); } -static struct v4l2_m2m_ops bdisp_m2m_ops = { +static const struct v4l2_m2m_ops bdisp_m2m_ops = { .device_run = bdisp_device_run, .job_abort = bdisp_job_abort, };
[PATCH 12/12] [media] mtk-mdp: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c index 3038d62..583d477 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c @@ -1225,7 +1225,7 @@ static int mtk_mdp_m2m_release(struct file *file) .mmap = v4l2_m2m_fop_mmap, }; -static struct v4l2_m2m_ops mtk_mdp_m2m_ops = { +static const struct v4l2_m2m_ops mtk_mdp_m2m_ops = { .device_run = mtk_mdp_m2m_device_run, .job_abort = mtk_mdp_m2m_job_abort, };
[PATCH 11/12] [media] exynos4-is: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/exynos4-is/fimc-m2m.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c index d8724fe..9027d0b 100644 --- a/drivers/media/platform/exynos4-is/fimc-m2m.c +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c @@ -704,7 +704,7 @@ static int fimc_m2m_release(struct file *file) .mmap = v4l2_m2m_fop_mmap, }; -static struct v4l2_m2m_ops m2m_ops = { +static const struct v4l2_m2m_ops m2m_ops = { .device_run = fimc_device_run, .job_abort = fimc_job_abort, };
[PATCH 09/12] [media] media: mx2-emmaprp: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/mx2_emmaprp.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c index 03e47e0..7fd209e 100644 --- a/drivers/media/platform/mx2_emmaprp.c +++ b/drivers/media/platform/mx2_emmaprp.c @@ -882,7 +882,7 @@ static int emmaprp_mmap(struct file *file, struct vm_area_struct *vma) .vfl_dir= VFL_DIR_M2M, }; -static struct v4l2_m2m_ops m2m_ops = { +static const struct v4l2_m2m_ops m2m_ops = { .device_run = emmaprp_device_run, .job_abort = emmaprp_job_abort, .lock = emmaprp_lock,
[PATCH 08/12] [media] m2m-deinterlace: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/m2m-deinterlace.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c index 980066b..98f6db2 100644 --- a/drivers/media/platform/m2m-deinterlace.c +++ b/drivers/media/platform/m2m-deinterlace.c @@ -988,7 +988,7 @@ static int deinterlace_mmap(struct file *file, struct vm_area_struct *vma) .vfl_dir= VFL_DIR_M2M, }; -static struct v4l2_m2m_ops m2m_ops = { +static const struct v4l2_m2m_ops m2m_ops = { .device_run = deinterlace_device_run, .job_ready = deinterlace_job_ready, .job_abort = deinterlace_job_abort,
[PATCH 10/12] [media] vim2m: constify v4l2_m2m_ops structures
The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct v4l2_m2m_ops i@p = { ... }; @ok1@ identifier r.i; position p; @@ v4l2_m2m_init(@p) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct v4l2_m2m_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct v4l2_m2m_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/vim2m.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c index 970b9b6..afbaa35 100644 --- a/drivers/media/platform/vim2m.c +++ b/drivers/media/platform/vim2m.c @@ -983,7 +983,7 @@ static int vim2m_release(struct file *file) .release= video_device_release_empty, }; -static struct v4l2_m2m_ops m2m_ops = { +static const struct v4l2_m2m_ops m2m_ops = { .device_run = device_run, .job_ready = job_ready, .job_abort = job_abort,
[PATCH 1/6] [media] v4l2-pci-skeleton: constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct vb2_ops i@p = { ... }; @ok@ identifier r.i; struct vb2_queue e; position p; @@ e.ops = @p; @bad@ position p != {r.p,ok.p}; identifier r.i; struct vb2_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct vb2_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- There doesn't seem to be a maintainer for this file. samples/v4l/v4l2-pci-skeleton.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/v4l/v4l2-pci-skeleton.c b/samples/v4l/v4l2-pci-skeleton.c index 93b76c3..483e9bca 100644 --- a/samples/v4l/v4l2-pci-skeleton.c +++ b/samples/v4l/v4l2-pci-skeleton.c @@ -282,7 +282,7 @@ static void stop_streaming(struct vb2_queue *vq) * vb2_ops_wait_prepare/finish helper functions. If q->lock would be NULL, * then this driver would have to provide these ops. */ -static struct vb2_ops skel_qops = { +static const struct vb2_ops skel_qops = { .queue_setup= queue_setup, .buf_prepare= buffer_prepare, .buf_queue = buffer_queue,
[PATCH 0/6] constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. --- drivers/media/platform/blackfin/bfin_capture.c|2 +- drivers/media/platform/davinci/vpbe_display.c |2 +- drivers/staging/media/davinci_vpfe/vpfe_video.c |2 +- drivers/staging/media/imx/imx-media-capture.c |4 ++-- drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c |2 +- samples/v4l/v4l2-pci-skeleton.c |2 +- 6 files changed, 7 insertions(+), 7 deletions(-)
[PATCH 2/6] [media] media: davinci: vpbe: constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct vb2_ops i@p = { ... }; @ok@ identifier r.i; struct vb2_queue e; position p; @@ e.ops = @p; @bad@ position p != {r.p,ok.p}; identifier r.i; struct vb2_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct vb2_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/davinci/vpbe_display.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c index ca2adfa..13d0270 100644 --- a/drivers/media/platform/davinci/vpbe_display.c +++ b/drivers/media/platform/davinci/vpbe_display.c @@ -355,7 +355,7 @@ static void vpbe_stop_streaming(struct vb2_queue *vq) spin_unlock_irqrestore(>dma_queue_lock, flags); } -static struct vb2_ops video_qops = { +static const struct vb2_ops video_qops = { .queue_setup = vpbe_buffer_queue_setup, .wait_prepare = vb2_ops_wait_prepare, .wait_finish = vb2_ops_wait_finish,
[PATCH 4/6] [media] media: blackfin: bfin_capture: constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct vb2_ops i@p = { ... }; @ok@ identifier r.i; struct vb2_queue e; position p; @@ e.ops = @p; @bad@ position p != {r.p,ok.p}; identifier r.i; struct vb2_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct vb2_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/platform/blackfin/bfin_capture.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c index 294d696..41f1791 100644 --- a/drivers/media/platform/blackfin/bfin_capture.c +++ b/drivers/media/platform/blackfin/bfin_capture.c @@ -375,7 +375,7 @@ static void bcap_stop_streaming(struct vb2_queue *vq) } } -static struct vb2_ops bcap_video_qops = { +static const struct vb2_ops bcap_video_qops = { .queue_setup= bcap_queue_setup, .buf_prepare= bcap_buffer_prepare, .buf_cleanup= bcap_buffer_cleanup,
[PATCH 6/6] [media] media: imx: capture: constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct vb2_ops i@p = { ... }; @ok@ identifier r.i; struct vb2_queue e; position p; @@ e.ops = @p; @bad@ position p != {r.p,ok.p}; identifier r.i; struct vb2_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct vb2_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/staging/media/imx/imx-media-capture.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c index ddab4c2..ea145ba 100644 --- a/drivers/staging/media/imx/imx-media-capture.c +++ b/drivers/staging/media/imx/imx-media-capture.c @@ -62,7 +62,7 @@ struct capture_priv { /* In bytes, per queue */ #define VID_MEM_LIMIT SZ_64M -static struct vb2_ops capture_qops; +static const struct vb2_ops capture_qops; /* * Video ioctls follow @@ -503,7 +503,7 @@ static void capture_stop_streaming(struct vb2_queue *vq) spin_unlock_irqrestore(>q_lock, flags); } -static struct vb2_ops capture_qops = { +static const struct vb2_ops capture_qops = { .queue_setup = capture_queue_setup, .buf_init= capture_buf_init, .buf_prepare = capture_buf_prepare,
[PATCH 3/6] [media] staging: media: davinci_vpfe: constify vb2_ops structures
These vb2_ops structures are only stored in the ops field of a vb2_queue structure, which is declared as const. Thus the vb2_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct vb2_ops i@p = { ... }; @ok@ identifier r.i; struct vb2_queue e; position p; @@ e.ops = @p; @bad@ position p != {r.p,ok.p}; identifier r.i; struct vb2_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct vb2_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/staging/media/davinci_vpfe/vpfe_video.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 8b2117e..155e8c7 100644 --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c @@ -1304,7 +1304,7 @@ static void vpfe_buf_cleanup(struct vb2_buffer *vb) list_del_init(>list); } -static struct vb2_ops video_qops = { +static const struct vb2_ops video_qops = { .queue_setup= vpfe_buffer_queue_setup, .buf_init = vpfe_buffer_init, .buf_prepare= vpfe_buffer_prepare,
[PATCH 1/5] [media] cx18: constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct videobuf_queue_ops i@p = { ... }; @ok1@ identifier r.i; expression e1; position p; @@ videobuf_queue_vmalloc_init(e1,@p,...) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct videobuf_queue_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct videobuf_queue_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/pci/cx18/cx18-streams.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/cx18/cx18-streams.c b/drivers/media/pci/cx18/cx18-streams.c index 3c45e007..81d06c1 100644 --- a/drivers/media/pci/cx18/cx18-streams.c +++ b/drivers/media/pci/cx18/cx18-streams.c @@ -240,7 +240,7 @@ static void buffer_queue(struct videobuf_queue *q, struct videobuf_buffer *vb) list_add_tail(>vb.queue, >vb_capture); } -static struct videobuf_queue_ops cx18_videobuf_qops = { +static const struct videobuf_queue_ops cx18_videobuf_qops = { .buf_setup= buffer_setup, .buf_prepare = buffer_prepare, .buf_queue= buffer_queue,
[PATCH 2/5] [media] atomisp: constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct videobuf_queue_ops i@p = { ... }; @ok1@ identifier r.i; expression e1; position p; @@ videobuf_queue_vmalloc_init(e1,@p,...) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct videobuf_queue_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct videobuf_queue_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c index c151c84..d8cfed3 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c @@ -643,14 +643,14 @@ static void atomisp_buf_release_output(struct videobuf_queue *vq, vb->state = VIDEOBUF_NEEDS_INIT; } -static struct videobuf_queue_ops videobuf_qops = { +static const struct videobuf_queue_ops videobuf_qops = { .buf_setup = atomisp_buf_setup, .buf_prepare= atomisp_buf_prepare, .buf_queue = atomisp_buf_queue, .buf_release= atomisp_buf_release, }; -static struct videobuf_queue_ops videobuf_qops_output = { +static const struct videobuf_queue_ops videobuf_qops_output = { .buf_setup = atomisp_buf_setup_output, .buf_prepare= atomisp_buf_prepare_output, .buf_queue = atomisp_buf_queue_output,
[PATCH 0/5] constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. --- drivers/media/pci/cx18/cx18-streams.c |2 +- drivers/media/usb/cx231xx/cx231xx-417.c |2 +- drivers/media/usb/cx231xx/cx231xx-video.c |2 +- drivers/media/usb/tm6000/tm6000-video.c |2 +- drivers/media/usb/zr364xx/zr364xx.c |2 +- drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c |4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-)
[PATCH 3/5] [media] cx231xx: constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct videobuf_queue_ops i@p = { ... }; @ok1@ identifier r.i; expression e1; position p; @@ videobuf_queue_vmalloc_init(e1,@p,...) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct videobuf_queue_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct videobuf_queue_ops i = { ... }; // In the first case, there is a second commented call to videobuf_queue_sg_init with the structure as the second argument. If that code will be uncommented, the const will remain correct, because the second parameter of that function is also const. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/cx231xx/cx231xx-417.c |2 +- drivers/media/usb/cx231xx/cx231xx-video.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c index 8d5eb99..d538fa4 100644 --- a/drivers/media/usb/cx231xx/cx231xx-417.c +++ b/drivers/media/usb/cx231xx/cx231xx-417.c @@ -1490,7 +1490,7 @@ static void bb_buf_release(struct videobuf_queue *q, free_buffer(q, buf); } -static struct videobuf_queue_ops cx231xx_qops = { +static const struct videobuf_queue_ops cx231xx_qops = { .buf_setup= bb_buf_setup, .buf_prepare = bb_buf_prepare, .buf_queue= bb_buf_queue, diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c index f67f868..179b848 100644 --- a/drivers/media/usb/cx231xx/cx231xx-video.c +++ b/drivers/media/usb/cx231xx/cx231xx-video.c @@ -859,7 +859,7 @@ static void buffer_release(struct videobuf_queue *vq, free_buffer(vq, buf); } -static struct videobuf_queue_ops cx231xx_video_qops = { +static const struct videobuf_queue_ops cx231xx_video_qops = { .buf_setup = buffer_setup, .buf_prepare = buffer_prepare, .buf_queue = buffer_queue,
[PATCH 4/5] [media] tm6000: constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct videobuf_queue_ops i@p = { ... }; @ok1@ identifier r.i; expression e1; position p; @@ videobuf_queue_vmalloc_init(e1,@p,...) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct videobuf_queue_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct videobuf_queue_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/tm6000/tm6000-video.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/tm6000/tm6000-video.c b/drivers/media/usb/tm6000/tm6000-video.c index cec1321..ec8c4d2 100644 --- a/drivers/media/usb/tm6000/tm6000-video.c +++ b/drivers/media/usb/tm6000/tm6000-video.c @@ -801,7 +801,7 @@ static void buffer_release(struct videobuf_queue *vq, struct videobuf_buffer *vb free_buffer(vq, buf); } -static struct videobuf_queue_ops tm6000_video_qops = { +static const struct videobuf_queue_ops tm6000_video_qops = { .buf_setup = buffer_setup, .buf_prepare= buffer_prepare, .buf_queue = buffer_queue,
[PATCH 5/5] [media] zr364xx: constify videobuf_queue_ops structures
These videobuf_queue_ops structures are only passed as the second argument to videobuf_queue_vmalloc_init, which is declared as const. Thus the videobuf_queue_ops structures themselves can be const. Done with the help of Coccinelle. // @r disable optional_qualifier@ identifier i; position p; @@ static struct videobuf_queue_ops i@p = { ... }; @ok1@ identifier r.i; expression e1; position p; @@ videobuf_queue_vmalloc_init(e1,@p,...) @bad@ position p != {r.p,ok1.p}; identifier r.i; struct videobuf_queue_ops e; @@ e@i@p @depends on !bad disable optional_qualifier@ identifier r.i; @@ static +const struct videobuf_queue_ops i = { ... }; // Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/media/usb/zr364xx/zr364xx.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/zr364xx/zr364xx.c b/drivers/media/usb/zr364xx/zr364xx.c index efdcd5b..24d5860 100644 --- a/drivers/media/usb/zr364xx/zr364xx.c +++ b/drivers/media/usb/zr364xx/zr364xx.c @@ -439,7 +439,7 @@ static void buffer_release(struct videobuf_queue *vq, free_buffer(vq, buf); } -static struct videobuf_queue_ops zr364xx_video_qops = { +static const struct videobuf_queue_ops zr364xx_video_qops = { .buf_setup = buffer_setup, .buf_prepare = buffer_prepare, .buf_queue = buffer_queue,
[PATCH] staging: media: use relevant lock
The data protected is video_out2 and the lock that is released is _out2->dma_queue_lock, so it seems that that lock should be taken as well. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/staging/media/davinci_vpfe/dm365_resizer.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c b/drivers/staging/media/davinci_vpfe/dm365_resizer.c index 857b0e8..4910cb7 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c @@ -1059,7 +1059,7 @@ static void resizer_ss_isr(struct vpfe_resizer_device *resizer) /* If resizer B is enabled */ if (pipe->output_num > 1 && resizer->resizer_b.output == RESIZER_OUTPUT_MEMORY) { - spin_lock(_out->dma_queue_lock); + spin_lock(_out2->dma_queue_lock); vpfe_video_process_buffer_complete(video_out2); video_out2->state = VPFE_VIDEO_BUFFER_NOT_QUEUED; vpfe_video_schedule_next_buffer(video_out2);
[PATCH] [media] DaVinci-VPBE: constify vpbe_dev_ops
vpbe_dev_ops is only copied into the ops field at the end of a vpbe_device structure, so it can be const. Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- Does the ops field need to be inlined into the vpbe_device structure? drivers/media/platform/davinci/vpbe.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c index 3679b1e..7f64625 100644 --- a/drivers/media/platform/davinci/vpbe.c +++ b/drivers/media/platform/davinci/vpbe.c @@ -790,7 +790,7 @@ static void vpbe_deinitialize(struct device *dev, struct vpbe_device *vpbe_dev) vpss_enable_clock(VPSS_VPBE_CLOCK, 0); } -static struct vpbe_device_ops vpbe_dev_ops = { +static const struct vpbe_device_ops vpbe_dev_ops = { .g_cropcap = vpbe_g_cropcap, .enum_outputs = vpbe_enum_outputs, .set_output = vpbe_set_output,
[ragnatech:media-tree 2075/2144] drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176 (fwd)
Is a lock release needed before line 1185? Is the !enable in line 1189 correct? If the code is correct as is, perhaps it could be good to add some comments. julia -- Forwarded message -- Date: Mon, 24 Jul 2017 12:55:30 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: [ragnatech:media-tree 2075/2144] drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176 CC: kbuild-...@01.org TO: Daniel Scheller <d.schel...@gmx.net> CC: Mauro Carvalho Chehab <m.che...@samsung.com> CC: linux-media@vger.kernel.org tree: git://git.ragnatech.se/linux media-tree head: 0e50e84a11f4854e9a7e3b7f4443ffb99e6be292 commit: cd21b334943719f880e707eb91895fc916a88000 [2075/2144] media: dvb-frontends: add ST STV0910 DVB-S/S2 demodulator frontend driver :: branch date: 3 days ago :: commit date: 4 days ago >> drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176 git remote add ragnatech git://git.ragnatech.se/linux git remote update ragnatech git checkout cd21b334943719f880e707eb91895fc916a88000 vim +1185 drivers/media/dvb-frontends/stv0910.c cd21b334 Daniel Scheller 2017-07-03 1168 cd21b334 Daniel Scheller 2017-07-03 1169 cd21b334 Daniel Scheller 2017-07-03 1170 static int gate_ctrl(struct dvb_frontend *fe, int enable) cd21b334 Daniel Scheller 2017-07-03 1171 { cd21b334 Daniel Scheller 2017-07-03 1172 struct stv *state = fe->demodulator_priv; cd21b334 Daniel Scheller 2017-07-03 1173 u8 i2crpt = state->i2crpt & ~0x86; cd21b334 Daniel Scheller 2017-07-03 1174 cd21b334 Daniel Scheller 2017-07-03 1175 if (enable) cd21b334 Daniel Scheller 2017-07-03 @1176 mutex_lock(>base->i2c_lock); cd21b334 Daniel Scheller 2017-07-03 1177 cd21b334 Daniel Scheller 2017-07-03 1178 if (enable) cd21b334 Daniel Scheller 2017-07-03 1179 i2crpt |= 0x80; cd21b334 Daniel Scheller 2017-07-03 1180 else cd21b334 Daniel Scheller 2017-07-03 1181 i2crpt |= 0x02; cd21b334 Daniel Scheller 2017-07-03 1182 cd21b334 Daniel Scheller 2017-07-03 1183 if (write_reg(state, state->nr ? RSTV0910_P2_I2CRPT : cd21b334 Daniel Scheller 2017-07-03 1184 RSTV0910_P1_I2CRPT, i2crpt) < 0) cd21b334 Daniel Scheller 2017-07-03 @1185 return -EIO; cd21b334 Daniel Scheller 2017-07-03 1186 cd21b334 Daniel Scheller 2017-07-03 1187 state->i2crpt = i2crpt; cd21b334 Daniel Scheller 2017-07-03 1188 cd21b334 Daniel Scheller 2017-07-03 1189 if (!enable) cd21b334 Daniel Scheller 2017-07-03 1190 mutex_unlock(>base->i2c_lock); cd21b334 Daniel Scheller 2017-07-03 1191 return 0; cd21b334 Daniel Scheller 2017-07-03 1192 } cd21b334 Daniel Scheller 2017-07-03 1193 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 1/3] [media] uvcvideo: variable size controls (fwd)
There is a double free on data if the goto is taken on line 1815. julia -- Forwarded message -- Date: Sat, 15 Jul 2017 21:07:03 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH 1/3] [media] uvcvideo: variable size controls CC: kbuild-...@01.org In-Reply-To: <20170714201424.23592-1-philipp.za...@gmail.com> TO: Philipp Zabel <philipp.za...@gmail.com> CC: linux-media@vger.kernel.org, Laurent Pinchart <laurent.pinch...@ideasonboard.com>, Philipp Zabel <philipp.za...@gmail.com> CC: Laurent Pinchart <laurent.pinch...@ideasonboard.com>, Philipp Zabel <philipp.za...@gmail.com> Hi Philipp, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12 next-20170714] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Philipp-Zabel/uvcvideo-variable-size-controls/20170715-193137 base: git://linuxtv.org/media_tree.git master :: branch date: 2 hours ago :: commit date: 2 hours ago >> drivers/media/usb/uvc/uvc_ctrl.c:1857:7-11: ERROR: reference preceded by >> free on line 1809 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout f06e94cde314fba5859cd6c999dde48e94156c7a vim +1857 drivers/media/usb/uvc/uvc_ctrl.c 52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 1719 8e113595e drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-07-01 1720 int uvc_xu_ctrl_query(struct uvc_video_chain *chain, fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1721 struct uvc_xu_control_query *xqry) c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1722 { c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1723 struct uvc_entity *entity; fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1724 struct uvc_control *ctrl; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1725 unsigned int i, found = 0; fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1726 __u32 reqflags; fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1727 __u16 size; fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1728 __u8 *data = NULL; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1729 int ret; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1730 c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1731 /* Find the extension unit. */ 6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25 1732 list_for_each_entry(entity, >entities, chain) { 6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25 1733 if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT && fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1734 entity->id == xqry->unit) c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1735 break; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1736 } c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1737 fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1738 if (entity->id != xqry->unit) { c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1739 uvc_trace(UVC_TRACE_CONTROL, "Extension unit %u not found.\n", fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1740 xqry->unit); fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1741 return -ENOENT; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1742 } c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1743 52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 1744 /* Find the control and perform delayed initialization if needed. */ c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1745 for (i = 0; i < entity->ncontrols; ++i) { c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1746 ctrl = >controls[i]; fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02 1747 if (ctrl->index == xqry->selector - 1) { c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30 1748 found = 1; c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent
Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver (fwd)
Please check on this (line 199). julia -- Forwarded message -- Date: Thu, 6 Jul 2017 13:58:29 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver Hi Maxime, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/dt-bindings-media-Add-Cadence-MIPI-CSI2RX-Device-Tree-bindings/20170705-205643 base: git://linuxtv.org/media_tree.git master :: branch date: 17 hours ago :: commit date: 17 hours ago >> drivers/media/platform/cadence/cdns-csi2rx.c:199:5-23: WARNING: Unsigned >> expression compared with zero: csi2rx -> sensor_pad < 0 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout fb3d2879ba0623bcdc83c99ba70229ec1713feaf vim +199 drivers/media/platform/cadence/cdns-csi2rx.c fb3d2879 Maxime Ripard 2017-07-03 183 return media_create_pad_link(>sensor_subdev->entity, fb3d2879 Maxime Ripard 2017-07-03 184 csi2rx->sensor_pad, fb3d2879 Maxime Ripard 2017-07-03 185 >subdev.entity, 0, fb3d2879 Maxime Ripard 2017-07-03 186 MEDIA_LNK_FL_ENABLED | fb3d2879 Maxime Ripard 2017-07-03 187 MEDIA_LNK_FL_IMMUTABLE); fb3d2879 Maxime Ripard 2017-07-03 188 } fb3d2879 Maxime Ripard 2017-07-03 189 fb3d2879 Maxime Ripard 2017-07-03 190 static int csi2rx_async_bound(struct v4l2_async_notifier *notifier, fb3d2879 Maxime Ripard 2017-07-03 191struct v4l2_subdev *subdev, fb3d2879 Maxime Ripard 2017-07-03 192struct v4l2_async_subdev *asd) fb3d2879 Maxime Ripard 2017-07-03 193 { fb3d2879 Maxime Ripard 2017-07-03 194 struct csi2rx_priv *csi2rx = v4l2_notifier_to_csi2rx(notifier); fb3d2879 Maxime Ripard 2017-07-03 195 fb3d2879 Maxime Ripard 2017-07-03 196 csi2rx->sensor_pad = media_entity_get_fwnode_pad(>entity, fb3d2879 Maxime Ripard 2017-07-03 197 >sensor_node->fwnode, fb3d2879 Maxime Ripard 2017-07-03 198 MEDIA_PAD_FL_SOURCE); fb3d2879 Maxime Ripard 2017-07-03 @199 if (csi2rx->sensor_pad < 0) { fb3d2879 Maxime Ripard 2017-07-03 200 dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n", fb3d2879 Maxime Ripard 2017-07-03 201 subdev->name); fb3d2879 Maxime Ripard 2017-07-03 202 return csi2rx->sensor_pad; fb3d2879 Maxime Ripard 2017-07-03 203 } fb3d2879 Maxime Ripard 2017-07-03 204 fb3d2879 Maxime Ripard 2017-07-03 205 csi2rx->sensor_subdev = subdev; fb3d2879 Maxime Ripard 2017-07-03 206 fb3d2879 Maxime Ripard 2017-07-03 207 dev_dbg(csi2rx->dev, "Bound %s pad: %d\n", subdev->name, --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support (fwd)
Braces are probably missing aroud lines 1618-1620. julia -- Forwarded message -- Date: Sun, 25 Jun 2017 23:06:03 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support Hi Hugues, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12-rc6 next-20170623] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153 base: git://linuxtv.org/media_tree.git master :: branch date: 3 hours ago :: commit date: 3 hours ago >> drivers/media/i2c/ov9650.c:1618:2-44: code aligned with following code on >> line 1619 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout a9fe8c23240a7f8df39c6238d98e41f41fedb641 vim +1618 drivers/media/i2c/ov9650.c a9fe8c23 Hugues Fruchet 2017-06-22 1612ov965x->set_params = __ov965x_set_params; 84a15ded Sylwester Nawrocki 2012-12-26 1613 a9fe8c23 Hugues Fruchet 2017-06-22 1614ov965x->frame_size = >framesizes[0]; a9fe8c23 Hugues Fruchet 2017-06-22 1615 ov965x_get_default_format(ov965x, >format); a9fe8c23 Hugues Fruchet 2017-06-22 1616 a9fe8c23 Hugues Fruchet 2017-06-22 1617if (ov965x->initialize_controls) a9fe8c23 Hugues Fruchet 2017-06-22 @1618ret = ov965x->initialize_controls(ov965x); 84a15ded Sylwester Nawrocki 2012-12-26 @1619if (ret < 0) 84a15ded Sylwester Nawrocki 2012-12-26 1620goto err_ctrls; 84a15ded Sylwester Nawrocki 2012-12-26 1621 84a15ded Sylwester Nawrocki 2012-12-26 1622/* Update exposure time min/max to match frame format */ --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
[ragnatech:media-tree 1869/1907] drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or | (fwd)
It seems that some of the constants on lines 3127 and on are the same as the ones on lines 3134 and on. julia -- Forwarded message -- Date: Wed, 21 Jun 2017 18:20:03 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: [ragnatech:media-tree 1869/1907] drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or | CC: kbuild-...@01.org TO: Daniel Scheller <d.schel...@gmx.net> CC: Mauro Carvalho Chehab <m.che...@samsung.com> CC: linux-media@vger.kernel.org tree: git://git.ragnatech.se/linux media-tree head: 76724b30f222067faf00874dc277f6c99d03d800 commit: dbbac11e1de1250ad39bbc15490c8614ac7f9def [1869/1907] [media] dvb-frontends/stv0367: add Digital Devices compatibility :: branch date: 20 hours ago :: commit date: 22 hours ago >> drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or >> | drivers/media/dvb-frontends/stv0367.c:3128:3-16: duplicated argument to & or | drivers/media/dvb-frontends/stv0367.c:3128:19-33: duplicated argument to & or | drivers/media/dvb-frontends/stv0367.c:3129:3-17: duplicated argument to & or | drivers/media/dvb-frontends/stv0367.c:3129:20-35: duplicated argument to & or | git remote add ragnatech git://git.ragnatech.se/linux git remote update ragnatech git checkout dbbac11e1de1250ad39bbc15490c8614ac7f9def vim +3127 drivers/media/dvb-frontends/stv0367.c dbbac11e Daniel Scheller 2017-03-29 3111 dbbac11e Daniel Scheller 2017-03-29 3112 return 0; dbbac11e Daniel Scheller 2017-03-29 3113 } dbbac11e Daniel Scheller 2017-03-29 3114 dbbac11e Daniel Scheller 2017-03-29 3115 static const struct dvb_frontend_ops stv0367ddb_ops = { dbbac11e Daniel Scheller 2017-03-29 3116 .delsys = { SYS_DVBC_ANNEX_A, SYS_DVBT }, dbbac11e Daniel Scheller 2017-03-29 3117 .info = { dbbac11e Daniel Scheller 2017-03-29 3118 .name = "ST STV0367 DDB DVB-C/T", dbbac11e Daniel Scheller 2017-03-29 3119 .frequency_min = 4700, dbbac11e Daniel Scheller 2017-03-29 3120 .frequency_max = 86500, dbbac11e Daniel Scheller 2017-03-29 3121 .frequency_stepsize = 17, dbbac11e Daniel Scheller 2017-03-29 3122 .frequency_tolerance = 0, dbbac11e Daniel Scheller 2017-03-29 3123 .symbol_rate_min = 87, dbbac11e Daniel Scheller 2017-03-29 3124 .symbol_rate_max = 1170, dbbac11e Daniel Scheller 2017-03-29 3125 .caps = /* DVB-C */ dbbac11e Daniel Scheller 2017-03-29 3126 0x400 |/* FE_CAN_QAM_4 */ dbbac11e Daniel Scheller 2017-03-29 @3127 FE_CAN_QAM_16 | FE_CAN_QAM_32 | dbbac11e Daniel Scheller 2017-03-29 3128 FE_CAN_QAM_64 | FE_CAN_QAM_128 | dbbac11e Daniel Scheller 2017-03-29 3129 FE_CAN_QAM_256 | FE_CAN_FEC_AUTO | dbbac11e Daniel Scheller 2017-03-29 3130 /* DVB-T */ dbbac11e Daniel Scheller 2017-03-29 3131 FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 | dbbac11e Daniel Scheller 2017-03-29 3132 FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | dbbac11e Daniel Scheller 2017-03-29 3133 FE_CAN_FEC_AUTO | dbbac11e Daniel Scheller 2017-03-29 3134 FE_CAN_QPSK | FE_CAN_QAM_16 | FE_CAN_QAM_64 | dbbac11e Daniel Scheller 2017-03-29 3135 FE_CAN_QAM_128 | FE_CAN_QAM_256 | FE_CAN_QAM_AUTO | --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v4 2/2] cec: add STM32 cec driver (fwd)
BRDNOGEN is duplicate in the #defined on line 46. julia -- Forwarded message -- Date: Mon, 29 May 2017 19:16:10 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH v4 2/2] cec: add STM32 cec driver CC: kbuild-...@01.org In-Reply-To: <1496046855-5809-3-git-send-email-benjamin.gaign...@linaro.org> TO: Benjamin Gaignard <benjamin.gaign...@linaro.org> CC: yannick.fer...@st.com, alexandre.tor...@st.com, hverk...@xs4all.nl, devicet...@vger.kernel.org, linux-media@vger.kernel.org, r...@kernel.org, hans.verk...@cisco.com CC: Benjamin Gaignard <benjamin.gaign...@linaro.org> Hi Benjamin, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12-rc3 next-20170529] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Benjamin-Gaignard/cec-STM32-driver/20170529-172722 base: git://linuxtv.org/media_tree.git master :: branch date: 2 hours ago :: commit date: 2 hours ago >> drivers/media/platform/stm32/stm32-cec.c:46:33-41: duplicated argument to & >> or | git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 8864245090acf32561bbec305dd8be5cfe31f1e1 vim +46 drivers/media/platform/stm32/stm32-cec.c 88642450 Benjamin Gaignard 2017-05-29 30 #define CEC_ISR 0x0010 /* Interrupt and status Register */ 88642450 Benjamin Gaignard 2017-05-29 31 #define CEC_IER 0x0014 /* Interrupt enable Register */ 88642450 Benjamin Gaignard 2017-05-29 32 88642450 Benjamin Gaignard 2017-05-29 33 #define TXEOMBIT(2) 88642450 Benjamin Gaignard 2017-05-29 34 #define TXSOMBIT(1) 88642450 Benjamin Gaignard 2017-05-29 35 #define CECENBIT(0) 88642450 Benjamin Gaignard 2017-05-29 36 88642450 Benjamin Gaignard 2017-05-29 37 #define LSTN BIT(31) 88642450 Benjamin Gaignard 2017-05-29 38 #define OAR GENMASK(30, 16) 88642450 Benjamin Gaignard 2017-05-29 39 #define SFTOPBIT(8) 88642450 Benjamin Gaignard 2017-05-29 40 #define BRDNOGEN BIT(7) 88642450 Benjamin Gaignard 2017-05-29 41 #define LBPEGEN BIT(6) 88642450 Benjamin Gaignard 2017-05-29 42 #define BREGEN BIT(5) 88642450 Benjamin Gaignard 2017-05-29 43 #define BRESTP BIT(4) 88642450 Benjamin Gaignard 2017-05-29 44 #define RXTOLBIT(3) 88642450 Benjamin Gaignard 2017-05-29 45 #define SFT GENMASK(2, 0) 88642450 Benjamin Gaignard 2017-05-29 @46 #define FULL_CFG (LSTN | SFTOP | BRDNOGEN | LBPEGEN | BREGEN | BRESTP \ 88642450 Benjamin Gaignard 2017-05-29 47| RXTOL | BRDNOGEN) 88642450 Benjamin Gaignard 2017-05-29 48 88642450 Benjamin Gaignard 2017-05-29 49 #define TXACKE BIT(12) 88642450 Benjamin Gaignard 2017-05-29 50 #define TXERRBIT(11) 88642450 Benjamin Gaignard 2017-05-29 51 #define TXUDRBIT(10) 88642450 Benjamin Gaignard 2017-05-29 52 #define TXENDBIT(9) 88642450 Benjamin Gaignard 2017-05-29 53 #define TXBR BIT(8) 88642450 Benjamin Gaignard 2017-05-29 54 #define ARBLST BIT(7) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
It looks like >cmdr_lock should be released at line 512 if it has not been released otherwise. The lock was taken at line 438. julia -- Forwarded message -- Date: Fri, 14 Apr 2017 22:21:44 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites Hi Logan, [auto build test WARNING on scsi/for-next] [also build test WARNING on v4.11-rc6] [cannot apply to next-20170413] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next :: branch date: 8 hours ago :: commit date: 8 hours ago >> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438 drivers/target/target_core_user.c:512:2-8: preceding lock on line 471 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 78082134e7afdc59d744eb8d2def5c588e89c378 vim +512 drivers/target/target_core_user.c 7c9e7a6f Andy Grover 2014-10-01 432 sizeof(struct tcmu_cmd_entry)); 7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size 7c9e7a6f Andy Grover 2014-10-01 434 + round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE); 7c9e7a6f Andy Grover 2014-10-01 435 7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size & (TCMU_OP_ALIGN_SIZE-1)); 7c9e7a6f Andy Grover 2014-10-01 437 7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 439 7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr; 7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */ 26418649 Sheng Yang 2016-02-26 442 data_length = se_cmd->data_length; 26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags & SCF_BIDI) { 26418649 Sheng Yang 2016-02-26 444 BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents)); 26418649 Sheng Yang 2016-02-26 445 data_length += se_cmd->t_bidi_data_sg->length; 26418649 Sheng Yang 2016-02-26 446 } 554617b2 Andy Grover 2016-08-25 447 if ((command_size > (udev->cmdr_size / 2)) || 554617b2 Andy Grover 2016-08-25 448 data_length > udev->data_size) { 554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request of size %zu/%zu is too big for %u/%zu " 3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data area\n", command_size, data_length, 7c9e7a6f Andy Grover 2014-10-01 451 udev->cmdr_size, udev->data_size); 554617b2 Andy Grover 2016-08-25 452 spin_unlock_irq(>cmdr_lock); 554617b2 Andy Grover 2016-08-25 453 return TCM_INVALID_CDB_FIELD; 554617b2 Andy Grover 2016-08-25 454 } 7c9e7a6f Andy Grover 2014-10-01 455 26418649 Sheng Yang 2016-02-26 456 while (!is_ring_space_avail(udev, command_size, data_length)) { 7c9e7a6f Andy Grover 2014-10-01 457 int ret; 7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait); 7c9e7a6f Andy Grover 2014-10-01 459 7c9e7a6f Andy Grover 2014-10-01 460 prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE); 7c9e7a6f Andy Grover 2014-10-01 461 7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for ring space\n"); 7c9e7a6f Andy Grover 2014-10-01 463 spin_unlock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 464 ret = schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT)); 7c9e7a6f Andy Grover 2014-10-01 465 finish_wait(>wait_cmdr, &__wait); 7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) { 7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu: command timed out\n"); 02eb924f Andy Grover 2016-10-06 468 return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; 7c9e7a6f Andy Grover 2014-10-01 469 } 7c9e7a6f Andy Grover 2014-10-01 470 7c9e7a6f Andy Grover 2014-10-01 471 spin_lock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 472 7c9e7a6f Andy Grover 2014-10-01 473 /* We dropped cmdr_lock, cmd_head is stale */ 7c9e7a6f Andy Grover 2014-10-01 474 cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */ 7c9e7a6f Andy Grover
Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder (fwd)
On Tue, 11 Apr 2017, Sylwester Nawrocki wrote: > Hi, > > On 04/03/2017 08:17 AM, Smitha T Murthy wrote: > > On Mon, 2017-04-03 at 08:00 +0200, Julia Lawall wrote: > > > See line 2101 > > > > > > julia > > > > > Thank you for bringing it to my notice, I had not checked on this git. > > I will upload the next version of patches soon corresponding to this > > git. > > In general please use the media master branch as a base for your patches > (git://linuxtv.org/media_tree.git master). Or latest branch in my > git repository, currently it's "for-v4.12/media/next-2" as can be seen > here: https://git.linuxtv.org/snawrocki/samsung.git I'm not making the patch. It comes to me from kbuild. If you would prefer some tree not to be included, you can notify Fengguang about this: fengguang...@intel.com julia > > -- > Thanks, > Sylwester >
Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder (fwd)
See line 2101 julia - Forwarded message -- Date: Mon, 3 Apr 2017 05:04:39 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder Hi Smitha, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.11-rc4 next-20170331] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Smitha-T-Murthy/Add-MFC-v10-10-support/20170403-033620 base: git://linuxtv.org/media_tree.git master :: branch date: 88 minutes ago :: commit date: 88 minutes ago >> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c:2101:6-25: WARNING: Unsigned >> expression compared with zero: p -> codec . hevc . level < 0 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout bca072db65317d79554527391338ce8bc6fbde58 vim +2101 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c bca072db Smitha T Murthy 2017-03-31 2085 break; bca072db Smitha T Murthy 2017-03-31 2086 case V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP: bca072db Smitha T Murthy 2017-03-31 2087 p->codec.hevc.rc_b_frame_qp = ctrl->val; bca072db Smitha T Murthy 2017-03-31 2088 break; bca072db Smitha T Murthy 2017-03-31 2089 case V4L2_CID_MPEG_VIDEO_HEVC_FRAME_RATE_RESOLUTION: bca072db Smitha T Murthy 2017-03-31 2090 p->codec.hevc.rc_framerate = ctrl->val; bca072db Smitha T Murthy 2017-03-31 2091 break; bca072db Smitha T Murthy 2017-03-31 2092 case V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP: bca072db Smitha T Murthy 2017-03-31 2093 p->codec.hevc.rc_min_qp = ctrl->val; bca072db Smitha T Murthy 2017-03-31 2094 break; bca072db Smitha T Murthy 2017-03-31 2095 case V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP: bca072db Smitha T Murthy 2017-03-31 2096 p->codec.hevc.rc_max_qp = ctrl->val; bca072db Smitha T Murthy 2017-03-31 2097 break; bca072db Smitha T Murthy 2017-03-31 2098 case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL: bca072db Smitha T Murthy 2017-03-31 2099 p->codec.hevc.level_v4l2 = ctrl->val; bca072db Smitha T Murthy 2017-03-31 2100 p->codec.hevc.level = hevc_level(ctrl->val); bca072db Smitha T Murthy 2017-03-31 @2101 if (p->codec.hevc.level < 0) { bca072db Smitha T Murthy 2017-03-31 2102 mfc_err("Level number is wrong\n"); bca072db Smitha T Murthy 2017-03-31 2103 ret = p->codec.hevc.level; bca072db Smitha T Murthy 2017-03-31 2104 } bca072db Smitha T Murthy 2017-03-31 2105 break; bca072db Smitha T Murthy 2017-03-31 2106 case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE: bca072db Smitha T Murthy 2017-03-31 2107 switch (ctrl->val) { bca072db Smitha T Murthy 2017-03-31 2108 case V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN: bca072db Smitha T Murthy 2017-03-31 2109 ctrl->val = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [Outreachy kernel] [PATCH v2] staging: media: davinci_vpfe:Replace a bit shift.
There should be a spac after every colon in the subject. Please pay attention to these small details, so you don't have to send the same patch over and over. julia On Fri, 24 Mar 2017, Arushi Singhal wrote: > This patch replaces bit shifting on 1 with the BIT(x) macro. > This was done with coccinelle: > @@ > constant c; > @@ > > -1 << c > +BIT(c) > > Signed-off-by: Arushi Singhal <arushisinghal19971...@gmail.com> > --- > changes in v2 > -Remove unnecessary parenthesis. > > drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 2 +- > drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +- > drivers/staging/media/davinci_vpfe/dm365_isif.c| 10 +- > drivers/staging/media/davinci_vpfe/dm365_resizer.c | 6 +++--- > 4 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c > b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c > index 6a3434cebd79..7eeb53217168 100644 > --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c > +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c > @@ -1815,7 +1815,7 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct > platform_device *pdev) > v4l2_subdev_init(sd, _v4l2_ops); > sd->internal_ops = _v4l2_internal_ops; > strlcpy(sd->name, "DAVINCI IPIPE", sizeof(sd->name)); > - sd->grp_id = 1 << 16; /* group ID for davinci subdevs */ > + sd->grp_id = BIT(16); /* group ID for davinci subdevs */ > v4l2_set_subdevdata(sd, ipipe); > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c > b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c > index 46fd2c7f69c3..c07f028dd6be 100644 > --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c > +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c > @@ -1021,7 +1021,7 @@ int vpfe_ipipeif_init(struct vpfe_ipipeif_device > *ipipeif, > > sd->internal_ops = _v4l2_internal_ops; > strlcpy(sd->name, "DAVINCI IPIPEIF", sizeof(sd->name)); > - sd->grp_id = 1 << 16; /* group ID for davinci subdevs */ > + sd->grp_id = BIT(16); /* group ID for davinci subdevs */ > > v4l2_set_subdevdata(sd, ipipeif); > > diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c > b/drivers/staging/media/davinci_vpfe/dm365_isif.c > index 569bcdc9ce2f..74b1247203b1 100644 > --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c > +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c > @@ -821,7 +821,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct > vpfe_isif_dfc *vdfc) > > /* Correct whole line or partial */ > if (vdfc->corr_whole_line) > - val |= 1 << ISIF_VDFC_CORR_WHOLE_LN_SHIFT; > + val |= BIT(ISIF_VDFC_CORR_WHOLE_LN_SHIFT); > > /* level shift value */ > val |= (vdfc->def_level_shift & ISIF_VDFC_LEVEL_SHFT_MASK) << > @@ -849,7 +849,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct > vpfe_isif_dfc *vdfc) > > val = isif_read(isif->isif_cfg.base_addr, DFCMEMCTL); > /* set DFCMARST and set DFCMWR */ > - val |= 1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT; > + val |= BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT); > val |= 1; > isif_write(isif->isif_cfg.base_addr, val, DFCMEMCTL); > > @@ -880,7 +880,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct > vpfe_isif_dfc *vdfc) > } > val = isif_read(isif->isif_cfg.base_addr, DFCMEMCTL); > /* clear DFCMARST and set DFCMWR */ > - val &= ~(1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT); > + val &= ~BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT); > val |= 1; > isif_write(isif->isif_cfg.base_addr, val, DFCMEMCTL); > > @@ -1140,7 +1140,7 @@ static int isif_config_raw(struct v4l2_subdev *sd, int > mode) > isif_write(isif->isif_cfg.base_addr, val, CGAMMAWD); > /* Configure DPCM compression settings */ > if (params->v4l2_pix_fmt == V4L2_PIX_FMT_SGRBG10DPCM8) { > - val = 1 << ISIF_DPCM_EN_SHIFT; > + val = BIT(ISIF_DPCM_EN_SHIFT); > val |= (params->dpcm_predictor & > ISIF_DPCM_PREDICTOR_MASK) << ISIF_DPCM_PREDICTOR_SHIFT; > } > @@ -2044,7 +2044,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, > struct platform_device *pdev) > v4l2_subdev_init(sd, _v4l2_ops); > sd->internal_ops = _v4l2_internal_ops; > strlcpy(sd->name, "DAVINCI ISIF", sizeof(sd->name)); > - sd->grp_i
Re: [Outreachy kernel] Re: [PATCH v1] staging: media: Remove unused function atomisp_set_stop_timeout()
On Sun, 12 Mar 2017, SIMRAN SINGHAL wrote: > On Sun, Mar 12, 2017 at 7:24 PM, Greg KH <gre...@linuxfoundation.org> wrote: > > On Fri, Mar 10, 2017 at 07:05:05PM +0530, simran singhal wrote: > >> The function atomisp_set_stop_timeout on being called, simply returns > >> back. The function hasn't been mentioned in the TODO and doesn't have > >> FIXME code around. Hence, atomisp_set_stop_timeout and its calls have been > >> removed. > >> > >> This was done using Coccinelle. > >> > >> @@ > >> identifier f; > >> @@ > >> > >> void f(...) { > >> > >> -return; > >> > >> } > >> > >> Signed-off-by: simran singhal <singhalsimr...@gmail.com> > >> --- > >> v1: > >>-Change Subject to include name of function > >>-change commit message to include the coccinelle script > > > > You should also cc: the developers doing all of the current work on this > > driver, Alan Cox, to get their comment if this really is something that > > can be removed or not. > > > > thanks, > > > Greg I have cc'd all the developers which script get_maintainer.pl showed: > > $ git show HEAD | perl scripts/get_maintainer.pl --separator , > --nokeywords --nogit --nogit-fallback --norolestats Sometimes people do a lot of work on something without being the maintainer. You can see who has done this using git log. Dropping some of the "no" arguments might give you the same information, but in practice the results tend to be an overapproximation... julia > > Mauro Carvalho Chehab <mche...@kernel.org>,Greg Kroah-Hartman > <gre...@linuxfoundation.org>, > linux-media@vger.kernel.org,de...@driverdev.osuosl.org,linux-ker...@vger.kernel.org > > > greg k-h > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyOdKmSF10Ba60_00OzzRMnKAt7XwjksmuQfGEKvBY-avg%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [PATCH] staging: atomisp: clean up return logic, remove redunant code
On Sun, 12 Mar 2017, walter harms wrote: > > > Am 11.03.2017 20:32, schrieb Colin King: > > From: Colin Ian King <colin.k...@canonical.com> > > > > There is no need to check if ret is non-zero, remove this > > redundant check and just return the error status from the call > > to mt9m114_write_reg_array. > > > > Detected by CoverityScan, CID#1416577 ("Identical code for > > different branches") > > > > Signed-off-by: Colin Ian King <colin.k...@canonical.com> > > --- > > drivers/staging/media/atomisp/i2c/mt9m114.c | 6 +- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c > > b/drivers/staging/media/atomisp/i2c/mt9m114.c > > index 8762124..a555aec 100644 > > --- a/drivers/staging/media/atomisp/i2c/mt9m114.c > > +++ b/drivers/staging/media/atomisp/i2c/mt9m114.c > > @@ -444,12 +444,8 @@ static int mt9m114_set_suspend(struct v4l2_subdev *sd) > > static int mt9m114_init_common(struct v4l2_subdev *sd) > > { > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > - int ret; > > > > - ret = mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING); > > - if (ret) > > - return ret; > > - return ret; > > + return mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING); > > } > > > any use for "client" ? I guess the code would be on two lines in any case. It looks like a nice decomposition as is. julia
Re: [Outreachy kernel] Re: [PATCH 1/7] staging: media: Remove unnecessary typecast of c90 int constant
On Fri, 3 Mar 2017, SIMRAN SINGHAL wrote: > On Fri, Mar 3, 2017 at 11:15 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote: > > Hi Simran, > > > > On Fri, Mar 03, 2017 at 01:21:56AM +0530, simran singhal wrote: > >> This patch removes unnecessary typecast of c90 int constant. > >> > >> WARNING: Unnecessary typecast of c90 int constant > >> > >> Signed-off-by: simran singhal <singhalsimr...@gmail.com> > > > > Which tree are these patches based on? > Can you please explain what's the problem with this patch. And > please elaborate your question. It is probably staging-testing. julia > > > > > -- > > Regards, > > > > Sakari Ailus > > e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyOeOK2k1K8Z2Yt3SmvJQ8A%2BvigNBsd39-paPwkRAY6CVQ%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. >
Re: [Outreachy kernel] [PATCH] staging: media: Remove parentheses from return arguments
On Fri, 3 Mar 2017, simran singhal wrote: > The sematic patch used for this is: > @@ > identifier i; > constant c; > @@ > return > - ( > \(i\|-i\|i(...)\|c\) > - ) > ; > > Signed-off-by: simran singhal <singhalsimr...@gmail.com> Acked-by: Julia Lawall <julia.law...@lip6.fr> > --- > .../media/atomisp/pci/atomisp2/css2400/sh_css.c | 20 > ++-- > .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 2 +- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > index f39d6f5..1216efb 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c > @@ -2009,7 +2009,7 @@ enum ia_css_err ia_css_suspend(void) > for(i=0;i<MAX_ACTIVE_STREAMS;i++) > ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "==*> after 1: seed %d > (%p)\n", i, my_css_save.stream_seeds[i].stream); > ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_suspend() leave\n"); > - return(IA_CSS_SUCCESS); > + return IA_CSS_SUCCESS; > } > > enum ia_css_err > @@ -2021,10 +2021,10 @@ ia_css_resume(void) > > err = ia_css_init(&(my_css_save.driver_env), my_css_save.loaded_fw, > my_css_save.mmu_base, my_css_save.irq_type); > if (err != IA_CSS_SUCCESS) > - return(err); > + return err; > err = ia_css_start_sp(); > if (err != IA_CSS_SUCCESS) > - return(err); > + return err; > my_css_save.mode = sh_css_mode_resume; > for(i=0;i<MAX_ACTIVE_STREAMS;i++) > { > @@ -2038,7 +2038,7 @@ ia_css_resume(void) > if (i) > for(j=0;j<i;j++) > > ia_css_stream_unload(my_css_save.stream_seeds[j].stream); > - return(err); > + return err; > } > err = > ia_css_stream_start(my_css_save.stream_seeds[i].stream); > if (err != IA_CSS_SUCCESS) > @@ -2048,7 +2048,7 @@ ia_css_resume(void) > > ia_css_stream_stop(my_css_save.stream_seeds[j].stream); > > ia_css_stream_unload(my_css_save.stream_seeds[j].stream); > } > - return(err); > + return err; > } > *my_css_save.stream_seeds[i].orig_stream = > my_css_save.stream_seeds[i].stream; > for(j=0;j<my_css_save.stream_seeds[i].num_pipes;j++) > @@ -2057,7 +2057,7 @@ ia_css_resume(void) > } > my_css_save.mode = sh_css_mode_working; > ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_resume() leave: > return_void\n"); > - return(IA_CSS_SUCCESS); > + return IA_CSS_SUCCESS; > } > > enum ia_css_err > @@ -10261,7 +10261,7 @@ ia_css_stream_load(struct ia_css_stream *stream) > for(k=0;k<j;k++) > > ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]); > } > - return(err); > + return err; > } > err = > ia_css_stream_create(&(my_css_save.stream_seeds[i].stream_config), > my_css_save.stream_seeds[i].num_pipes, > > my_css_save.stream_seeds[i].pipes, &(my_css_save.stream_seeds[i].stream)); > @@ -10270,12 +10270,12 @@ ia_css_stream_load(struct ia_css_stream *stream) > ia_css_stream_destroy(stream); > > for(j=0;j<my_css_save.stream_seeds[i].num_pipes;j++) > > ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]); > - return(err); > + return err; > } > break; > } > ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_stream_load() exit, > \n"); > - return(IA_CSS_SUCCESS); > + return IA_CSS_SUCCESS; > #else > /* TODO remove function - DEPRECATED */ > (void)stream; > @@ -10416,7 +10416,7 @@ ia_css_stream_unload(struct ia_css_stream *stream) >
Re: [patch] [media] uvcvideo: freeing an error pointer
To put it another way, tools can figure out what is missing if they have access to good exmples of what should be there. To be clear, I can see both points of view. julia -- 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
Re: [patch] [media] uvcvideo: freeing an error pointer
On Mon, 28 Nov 2016, Dan Carpenter wrote: > I understand the comparison, but I just think it's better if people > always keep track of what has been allocated and what has not. I tried > so hard to get Markus to stop sending those hundreds of patches where > he's like "this function has a sanity check so we can pass pointers > that weren't allocated"... It's garbage code. > > But I understand that other people don't agree. In my opinion, it is good for code understanding to only do what is useful to do. It's not a hard and fast rule, but I think it is something to take into account. julia > > regards, > dan carpenter > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
question about hva_hw_probe
The function hva_hw_probe in the file drivers/media/platform/sti/hva/hva-hw.c contains the following code: /* get memory for esram */ esram = platform_get_resource(pdev, IORESOURCE_MEM, 1); if (IS_ERR_OR_NULL(esram)) { dev_err(dev, "%s failed to get esram\n", HVA_PREFIX); return PTR_ERR(esram); } hva->esram_addr = esram->start; hva->esram_size = resource_size(esram); platform_get_resource only returns NULL on failure, so the test of IS_ERR_OR_NULL doesn't look appropriate. Nor does the return value of PTR_ERR(esram), which will be 0 on a NULL result. But I wondered if it was intended to have a call to devm_ioremap_resource between the platform_get_resource and the IS_ERR_OR_NULL test (which should be just IS_ERR; likewise for the call just above)? thanks, julia -- 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
Re: [PATCH] Staging:media:davinci_vpfe: used devm_kzalloc in place of kzalloc
On Wed, 2 Nov 2016, Nadim Almas wrote: > Switch to resource-managed function devm_kzalloc instead > of kzolloc and remove unneeded kzfree > > Also, remove kzfree in probe function and remove > function,vpfe_remove as it is now has nothing to do. > The Coccinelle semantic patch used to make this change is as follows: > / > @platform@ > identifier p, probefn, removefn; > @@ > struct platform_driver p = { > .probe = probefn, > .remove = removefn, > }; > > @prb@ > identifier platform.probefn, pdev; > expression e, e1, e2; > @@ > probefn(struct platform_device *pdev, ...) { > <+... > - e = kzalloc(e1, e2) > + e = devm_kzalloc(>dev, e1, e2) > ... > ?-kzfree(e); > ...+> > } > @rem depends on prb@ > identifier platform.removefn; > expression prb.e; > @@ > removefn(...) { > <... > - kzfree(e); > ...> > } > // > > Signed-off-by: Nadim Almas <nadim@gmail.com> > --- > drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c > b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c > index bf077f8..cd44f0f 100644 > --- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c > +++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c > @@ -579,7 +579,7 @@ static int vpfe_probe(struct platform_device *pdev) > struct resource *res1; > int ret = -ENOMEM; > > - vpfe_dev = kzalloc(sizeof(*vpfe_dev), GFP_KERNEL); > + vpfe_dev = devm_kzalloc(>dev, sizeof(*vpfe_dev), GFP_KERNEL); > if (!vpfe_dev) > return ret; > > @@ -681,7 +681,6 @@ static int vpfe_probe(struct platform_device *pdev) > probe_disable_clock: > vpfe_disable_clock(vpfe_dev); > probe_free_dev_mem: > - kzfree(vpfe_dev); Kzfree zeroes the data before freeing it. Devm_kzalloc only causes a kfree to happen, not a kzfree. If the kzfree is needed, then a memset 0 would need to replace the calls to kzfree. There are some other minor issues with the patch. In the subject line, there is normally a space after each :. There is a spelling mistake in the commit message. In the commit message there should be one space betwee words within a sentence, and there should be a space after a comma, or other puctuation. julia > > return ret; > } > @@ -702,7 +701,6 @@ static int vpfe_remove(struct platform_device *pdev) > v4l2_device_unregister(_dev->v4l2_dev); > media_device_unregister(_dev->media_dev); > vpfe_disable_clock(vpfe_dev); > - kzfree(vpfe_dev); > > return 0; > } > -- > 2.7.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
Re: [PATCH 1/3] [media] au0828-video: Use kcalloc() in au0828_init_isoc()
On Mon, 24 Oct 2016, Mauro Carvalho Chehab wrote: > Em Mon, 24 Oct 2016 23:28:44 +0100 > Andrey Utkin <andrey_ut...@fastmail.com> escreveu: > > > On Mon, Oct 24, 2016 at 10:59:24PM +0200, SF Markus Elfring wrote: > > > From: Markus Elfring <elfr...@users.sourceforge.net> > > > Date: Mon, 24 Oct 2016 22:08:47 +0200 > > > > > > * Multiplications for the size determination of memory allocations > > > indicated that array data structures should be processed. > > > Thus use the corresponding function "kcalloc". > > > > > > This issue was detected by using the Coccinelle software. > > > > > > * Replace the specification of data types by pointer dereferences > > > to make the corresponding size determination a bit safer according to > > > the Linux coding style convention. > > > > > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> > > > --- > > > drivers/media/usb/au0828/au0828-video.c | 11 +++ > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/media/usb/au0828/au0828-video.c > > > b/drivers/media/usb/au0828/au0828-video.c > > > index 85dd9a8..85b13c1 100644 > > > --- a/drivers/media/usb/au0828/au0828-video.c > > > +++ b/drivers/media/usb/au0828/au0828-video.c > > > @@ -221,15 +221,18 @@ static int au0828_init_isoc(struct au0828_dev *dev, > > > int max_packets, > > > > > > dev->isoc_ctl.isoc_copy = isoc_copy; > > > dev->isoc_ctl.num_bufs = num_bufs; > > > - > > > > > - dev->isoc_ctl.urb = kzalloc(sizeof(void *)*num_bufs, GFP_KERNEL); > > > + dev->isoc_ctl.urb = kcalloc(num_bufs, > > > + sizeof(*dev->isoc_ctl.urb), > > > + GFP_KERNEL); > > > > What about this (for both hunks)? > > > > - dev->isoc_ctl.urb = kzalloc(sizeof(void *)*num_bufs, GFP_KERNEL); > > + dev->isoc_ctl.urb = > > + kcalloc(num_bufs, sizeof(*dev->isoc_ctl.urb), GFP_KERNEL); > > > That's worse :) > > The usual Kernel style is: > > var = foo(bar1, > bar2, > bar3); Isn't it more like var = foo(bar1, bar2, bar3) Otherwise, Markus is going to send millions of patches to put every function argument on its own line... julia > > instead of something like: > > var = > foo(bar1, > bar2, > bar3); > > The places where it is different than that is because people ran > ./scripts/Lindent to try to follow the Kernel coding style. > > On my experiences, at the end, using it caused more harm than > good, IMHO, and cause very weird indentation on lines with > more than 80 columns like the above. > > Thanks, > Mauro > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
Re: [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169
On Tue, 11 Oct 2016, Mauro Carvalho Chehab wrote: > Em Tue, 11 Oct 2016 15:16:24 +0200 (CEST) > Julia Lawall <julia.law...@lip6.fr> escreveu: > > > On Tue, 11 Oct 2016, Julia Lawall wrote: > > > > > It looks like a lock may be needed before line 174. > > > > Sorry, an unlock. > > I suspect that this is a false positive warning, as there is a > mutex unlock on the same routine, at line 203. All exit > conditions go to the unlock condition. There is a direct exit in line 174. julia > > Am I missing something? > > > > > > > > > julia > > > > > > -- Forwarded message -- > > > Date: Tue, 11 Oct 2016 21:06:18 +0800 > > > From: kbuild test robot <fengguang...@intel.com> > > > To: kbu...@01.org > > > Cc: Julia Lawall <julia.law...@lip6.fr> > > > Subject: > > > > > > [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi > > > a-usb-drivers/20161011-182408 3/31] > > > drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on > > > line > > > 169 > > > > > > CC: kbuild-...@01.org > > > TO: Mauro Carvalho Chehab <m.che...@samsung.com> > > > CC: linux-media@vger.kernel.org > > > CC: 0day robot <fengguang...@intel.com> > > > > > > tree: https://github.com/0day-ci/linux > > > Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 > > > head: ff49f775552fe4ebe2944527cf882073679cb1e5 > > > commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: > > > handle error code on RC query > > > :: branch date: 3 hours ago > > > :: commit date: 3 hours ago > > > > > > >> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on > > > >> line 169 > > > > > > git remote add linux-review https://github.com/0day-ci/linux > > > git remote update linux-review > > > git checkout b38d98275e144aaea9db69ba2dcba58466046d9b > > > vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c > > > > > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > > > 2008-09-19 163 { > > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > > > 2008-09-19 164 struct cinergyt2_state *st = d->priv; > > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 165 int i, ret; > > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > > > 2008-09-19 166 > > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > > > 2008-09-19 167 *state = REMOTE_NO_KEY_PRESSED; > > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > > > 2008-09-19 168 > > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 @169 mutex_lock(>data_mutex); > > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 170 st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; > > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 171 > > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 172 ret = dvb_usb_generic_rw(d, st->data, 1, > > > st->data, 5, 0); > > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 173 if (ret < 0) > > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 @174 return ret; > > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 175 > > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > > > 2016-10-11 176 if (st->data[4] == 0xff) { > > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > > > 2008-09-19 177 /* key repeat */ > > > > > > --- > > > 0-DAY kernel test infrastructureOpen Source Technology > > > Center > > > https://lists.01.org/pipermail/kbuild-all Intel > > > Corporation > > > > > > > > > > -- > Thanks, > Mauro > -- 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
Re: [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169
On Tue, 11 Oct 2016, Julia Lawall wrote: > It looks like a lock may be needed before line 174. Sorry, an unlock. > > julia > > -- Forwarded message -- > Date: Tue, 11 Oct 2016 21:06:18 +0800 > From: kbuild test robot <fengguang...@intel.com> > To: kbu...@01.org > Cc: Julia Lawall <julia.law...@lip6.fr> > Subject: > > [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi > a-usb-drivers/20161011-182408 3/31] > drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line > 169 > > CC: kbuild-...@01.org > TO: Mauro Carvalho Chehab <m.che...@samsung.com> > CC: linux-media@vger.kernel.org > CC: 0day robot <fengguang...@intel.com> > > tree: https://github.com/0day-ci/linux > Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 > head: ff49f775552fe4ebe2944527cf882073679cb1e5 > commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: > handle error code on RC query > :: branch date: 3 hours ago > :: commit date: 3 hours ago > > >> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line > >> 169 > > git remote add linux-review https://github.com/0day-ci/linux > git remote update linux-review > git checkout b38d98275e144aaea9db69ba2dcba58466046d9b > vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > 2008-09-19 163 { > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > 2008-09-19 164 struct cinergyt2_state *st = d->priv; > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 165 int i, ret; > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > 2008-09-19 166 > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > 2008-09-19 167 *state = REMOTE_NO_KEY_PRESSED; > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava > 2008-09-19 168 > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 @169 mutex_lock(>data_mutex); > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 170 st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 171 > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 172 ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 173 if (ret < 0) > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 @174 return ret; > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 175 > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab > 2016-10-11 176 if (st->data[4] == 0xff) { > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE > 2008-09-19 177 /* key repeat */ > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation > -- 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
[linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169
It looks like a lock may be needed before line 174. julia -- Forwarded message -- Date: Tue, 11 Oct 2016 21:06:18 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi a-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169 CC: kbuild-...@01.org TO: Mauro Carvalho Chehab <m.che...@samsung.com> CC: linux-media@vger.kernel.org CC: 0day robot <fengguang...@intel.com> tree: https://github.com/0day-ci/linux Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 head: ff49f775552fe4ebe2944527cf882073679cb1e5 commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: handle error code on RC query :: branch date: 3 hours ago :: commit date: 3 hours ago >> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line >> 169 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout b38d98275e144aaea9db69ba2dcba58466046d9b vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava 2008-09-19 163 { 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 2008-09-19 164 struct cinergyt2_state *st = d->priv; b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 165 int i, ret; 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 2008-09-19 166 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava 2008-09-19 167 *state = REMOTE_NO_KEY_PRESSED; 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava 2008-09-19 168 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 @169 mutex_lock(>data_mutex); 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 170 st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS; 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 171 b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 172 ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0); b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 173 if (ret < 0) b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 @174 return ret; 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 175 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 2016-10-11 176 if (st->data[4] == 0xff) { 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 2008-09-19 177 /* key repeat */ --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- 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
Re: kernel-doc-rst-lint (was: Re: [PATCH 00/15] improve function-level documentation)
On Wed, 5 Oct 2016, Jani Nikula wrote: > On Wed, 05 Oct 2016, Daniel Vetter <dan...@ffwll.ch> wrote: > > Jani Nikula has a patch with a scrip to make the one kernel-doc parser > > into a lint/checker pass over the entire kernel. I think that'd would > > be more robust instead of trying to approximate the real kerneldoc > > parser. Otoh that parser is a horror show of a perl/regex driven state > > machine ;-) > > > > Jani, can you pls digg out these patches? Can't find them right now ... > > Expanding the massive Cc: with linux-doc list... > > Here goes. It's a quick hack from months ago, but still seems to > somewhat work. At least for the kernel-doc parts. The reStructuredText > lint part isn't all that great, and doesn't have mapping to line numbers > like the Sphinx kernel-doc extension does. Anyway I'm happy how this > integrates with kernel build CHECK and C=1/C=2. > > I guess Julia's goal is to automate the *fixing* of some of the error > classes from kernel-doc. Not sure how well this could be made to > integrate with any of that. No, my work doesn't fix anything. Coccinelle can't actually process comments. I just correlated the parsed comment with the function header. julia > > BR, > Jani. > > > From 1244efa0f63a7b13795e8c37f81733a3c8bfc56a Mon Sep 17 00:00:00 2001 > From: Jani Nikula <jani.nik...@intel.com> > Date: Tue, 31 May 2016 18:11:33 +0300 > Subject: [PATCH] kernel-doc-rst-lint: add tool to check kernel-doc and rst > correctness > Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo > Cc: Jani Nikula <jani.nik...@intel.com> > > Simple kernel-doc and reStructuredText lint tool that can be used > independently and as a kernel build CHECK tool to validate kernel-doc > comments. > > Independent usage: > $ kernel-doc-rst-lint FILE > > Kernel CHECK usage: > $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2) > > Depends on docutils and the rst-lint package > https://pypi.python.org/pypi/restructuredtext_lint > > Signed-off-by: Jani Nikula <jani.nik...@intel.com> > --- > scripts/kernel-doc-rst-lint | 106 > > 1 file changed, 106 insertions(+) > create mode 100755 scripts/kernel-doc-rst-lint > > diff --git a/scripts/kernel-doc-rst-lint b/scripts/kernel-doc-rst-lint > new file mode 100755 > index ..7e0157679f83 > --- /dev/null > +++ b/scripts/kernel-doc-rst-lint > @@ -0,0 +1,106 @@ > +#!/usr/bin/env python > +# coding=utf-8 > +# > +# Copyright © 2016 Intel Corporation > +# > +# Permission is hereby granted, free of charge, to any person obtaining a > +# copy of this software and associated documentation files (the "Software"), > +# to deal in the Software without restriction, including without limitation > +# the rights to use, copy, modify, merge, publish, distribute, sublicense, > +# and/or sell copies of the Software, and to permit persons to whom the > +# Software is furnished to do so, subject to the following conditions: > +# > +# The above copyright notice and this permission notice (including the next > +# paragraph) shall be included in all copies or substantial portions of the > +# Software. > +# > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > +# IN THE SOFTWARE. > +# > +# Authors: > +#Jani Nikula <jani.nik...@intel.com> > +# > +# Simple kernel-doc and reStructuredText lint tool that can be used > +# independently and as a kernel build CHECK tool to validate kernel-doc > +# comments. > +# > +# Independent usage: > +# $ kernel-doc-rst-lint FILE > +# > +# Kernel CHECK usage: > +# $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2) > +# > +# Depends on docutils and the rst-lint package > +# https://pypi.python.org/pypi/restructuredtext_lint > +# > + > +import os > +import subprocess > +import sys > + > +from docutils.parsers.rst import directives > +from docutils.parsers.rst import Directive > +from docutils.parsers.rst import roles > +from docutils import nodes, statemachine > +import restructuredtext_lint > + > +class DummyDirective(Directive): > +required_argument = 1 > +optional_arguments = 0 > +option
Re: [PATCH 00/15] improve function-level documentation
On Wed, 5 Oct 2016, Daniel Vetter wrote: > Jani Nikula has a patch with a scrip to make the one kernel-doc parser > into a lint/checker pass over the entire kernel. I think that'd would > be more robust instead of trying to approximate the real kerneldoc > parser. Otoh that parser is a horror show of a perl/regex driven state > machine ;-) Sure. To my recollection, I found around 2000 issues. Many I ignored, eg functions that simply have no documentation abuot the parameters, functions that document their local variables, when these were more interesting than the parameters etc. But the set of patches is not exhaustive with respect to the remaining interesting ones either. julia > > Jani, can you pls digg out these patches? Can't find them right now ... > -Daniel > > > On Sat, Oct 1, 2016 at 9:46 PM, Julia Lawall <julia.law...@lip6.fr> wrote: > > These patches fix cases where the documentation above a function definition > > is not consistent with the function header. Issues are detected using the > > semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic > > patch parses a file to find comments, then matches each function header, > > and checks that the name and parameter list in the function header are > > compatible with the comment that preceeds it most closely. > > > > // > > @initialize:ocaml@ > > @@ > > > > let tbl = ref [] > > let fnstart = ref [] > > let success = Hashtbl.create 101 > > let thefile = ref "" > > let parsed = ref [] > > let nea = ref [] > > > > let parse file = > > thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; > > let i = open_in file in > > let startline = ref 0 in > > let fn = ref "" in > > let ids = ref [] in > > let rec inside n = > > let l = input_line i in > > let n = n + 1 in > > match Str.split_delim (Str.regexp_string "*/") l with > > before::after::_ -> > > (if not (!fn = "") > > then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); > > startline := 0; > > fn := ""; > > ids := []; > > outside n > > | _ -> > > (match Str.split (Str.regexp "[ \t]+") l with > > "*"::name::rest -> > > let len = String.length name in > > (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" > > then fn := String.sub name 0 (len-2) > > else if !fn = "" && (not (rest = [])) && List.hd rest = "-" > > then > > if String.get name (len-1) = ':' > > then fn := String.sub name 0 (len-1) > > else fn := name > > else if not(!fn = "") && len > 2 && > > String.get name 0 = '@' && String.get name (len-1) = ':' > > then ids := (String.sub name 1 (len-2)) :: !ids); > > | _ -> ()); > > inside n > > and outside n = > > let l = input_line i in > > let n = n + 1 in > > if String.length l > 2 && String.sub l 0 3 = "/**" > > then > > begin > > startline := n; > > inside n > > end > > else outside n in > > try outside 0 with End_of_file -> () > > > > let hashadd tbl k v = > > let cell = > > try Hashtbl.find tbl k > > with Not_found -> > > let cell = ref [] in > > Hashtbl.add tbl k cell; > > cell in > > cell := v :: !cell > > > > @script:ocaml@ > > @@ > > > > tbl := []; > > fnstart := []; > > Hashtbl.clear success; > > parsed := []; > > nea := []; > > parse (List.hd (Coccilib.files())) > > > > @r@ > > identifier f; > > position p; > > @@ > > > > f@p(...) { ... } > > > > @script:ocaml@ > > p << r.p; > > f << r.f; > > @@ > > > > parsed := f :: !parsed; > > fnstart := (List.hd p).line :: !fnstart > > > > @param@ > > identifier f; > > type T; > > identifier i; > > parameter list[n] ps; > > parameter list[n1] ps1; > > position p; > > @@ > > > > f@p(ps,T i,ps1) { ... } > > > > @script:ocaml@ > > @@ > > > > tbl := List.rev (List.sort compare !tbl) > > > > @script:ocaml@ > &
Re: [PATCH 00/15] improve function-level documentation
On Sat, 1 Oct 2016, Joe Perches wrote: > On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote: > > These patches fix cases where the documentation above a function definition > > is not consistent with the function header. Issues are detected using the > > semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic > > patch parses a file to find comments, then matches each function header, > > and checks that the name and parameter list in the function header are > > compatible with the comment that preceeds it most closely. > > Hi Julia. > > Would it be possible for a semantic patch to scan for > function definitions where the types do not have > identifiers and update the definitions to match the > declarations? > > For instance, given: > > > int foo(int); > > > int foo(int bar) > { > return baz; > } > > Could coccinelle output: > > diff a/some.h b/some.h > [] > -int foo(int); > +int foo(int bar); The following seems to work: @r@ identifier f; position p; type T, t; parameter list[n] ps; @@ T f@p(ps,t,...); @s@ identifier r.f,x; type r.T, r.t; parameter list[r.n] ps; @@ T f(ps,t x,...) { ... } @@ identifier r.f, s.x; position r.p; type r.T, r.t; parameter list[r.n] ps; @@ T f@p(ps,t + x ,...); After letting it run for a few minutes without making any effort to include .h files, I get over 2700 changed lines. julia -- 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 00/15] improve function-level documentation
These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely. // @initialize:ocaml@ @@ let tbl = ref [] let fnstart = ref [] let success = Hashtbl.create 101 let thefile = ref "" let parsed = ref [] let nea = ref [] let parse file = thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; let i = open_in file in let startline = ref 0 in let fn = ref "" in let ids = ref [] in let rec inside n = let l = input_line i in let n = n + 1 in match Str.split_delim (Str.regexp_string "*/") l with before::after::_ -> (if not (!fn = "") then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); startline := 0; fn := ""; ids := []; outside n | _ -> (match Str.split (Str.regexp "[ \t]+") l with "*"::name::rest -> let len = String.length name in (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" then fn := String.sub name 0 (len-2) else if !fn = "" && (not (rest = [])) && List.hd rest = "-" then if String.get name (len-1) = ':' then fn := String.sub name 0 (len-1) else fn := name else if not(!fn = "") && len > 2 && String.get name 0 = '@' && String.get name (len-1) = ':' then ids := (String.sub name 1 (len-2)) :: !ids); | _ -> ()); inside n and outside n = let l = input_line i in let n = n + 1 in if String.length l > 2 && String.sub l 0 3 = "/**" then begin startline := n; inside n end else outside n in try outside 0 with End_of_file -> () let hashadd tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref [] in Hashtbl.add tbl k cell; cell in cell := v :: !cell @script:ocaml@ @@ tbl := []; fnstart := []; Hashtbl.clear success; parsed := []; nea := []; parse (List.hd (Coccilib.files())) @r@ identifier f; position p; @@ f@p(...) { ... } @script:ocaml@ p << r.p; f << r.f; @@ parsed := f :: !parsed; fnstart := (List.hd p).line :: !fnstart @param@ identifier f; type T; identifier i; parameter list[n] ps; parameter list[n1] ps1; position p; @@ f@p(ps,T i,ps1) { ... } @script:ocaml@ @@ tbl := List.rev (List.sort compare !tbl) @script:ocaml@ p << param.p; f << param.f; @@ let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if not (String.lowercase f = String.lowercase nm) then Printf.printf "%s:%d %s doesn't match preceding comment: %s\n" !thefile myline f nm); true end else false) !tbl in () @script:ocaml@ p << param.p; n << param.n; n1 << param.n1; i << param.i; f << param.f; @@ let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if List.mem i ids then hashadd success (st,fn,nm) i); (if ids = [] (* arg list seems not obligatory *) then () else if not (List.mem i ids) then Printf.printf "%s:%d %s doesn't appear in ids: %s\n" !thefile myline i (String.concat " " ids) else if List.length ids <= n || List.length ids <= n1 then (if not (List.mem f !nea) then begin nea := f :: !nea; Printf.printf "%s:%d %s not enough args\n" !thefile myline f; end) else let foundid = List.nth ids n in let efoundid = List.nth (List.rev ids) n1 in if not(foundid = i || efoundid = i) then Printf.printf "%s:%d %s wrong arg in position %d: %s\n" !thefile myline i n foundid); true end else false) !tbl in () @script:ocaml@ @@ List.iter (function (st,fn,nm,ids) -> if List.mem nm !parsed then let entry = try !(Hashtbl.find success (st,fn,nm)) with Not_found -> [] in List.iter (fun id -> if not (List.mem id entry) && not (id = "...") then Printf.printf "%s:%d %s not used\n"
[PATCH 05/15] dma-buf/sw_sync: improve function-level documentation
Adjust the documentation to use the names that appear in the function parameter list. Issue detected using Coccinelle (http://coccinelle.lip6.fr/) Signed-off-by: Julia Lawall <julia.law...@lip6.fr> --- drivers/dma-buf/sw_sync.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 62e8e6d..5d2b1b6 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -155,11 +155,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) /** * sync_pt_create() - creates a sync pt - * @parent:fence's parent sync_timeline + * @obj: fence's parent sync_timeline * @size: size to allocate for this pt - * @inc: value of the fence + * @value: value of the fence * - * Creates a new sync_pt as a child of @parent. @size bytes will be + * Creates a new sync_pt as a child of @obj. @size bytes will be * allocated allowing for implementation specific data to be kept after * the generic sync_timeline struct. Returns the sync_pt object or * NULL in case of error. -- 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
Re: [PATCH 00/26] constify local structures
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote: > On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote: > > > > Hi, > > > > Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes: > > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote: > > >> > > >> > > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote: > > >> > > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote: > > >> > > Constify local structures. > > >> > > > > >> > > The semantic patch that makes this change is as follows: > > >> > > (http://coccinelle.lip6.fr/) > > >> > > > >> > Just my two cents but: > > >> > > > >> > 1. You *can* use a static analysis too to find bugs or other issues. > > >> > 2. However, you should manually do the commits and proper commit > > >> >messages to subsystems based on your findings. And I generally think > > >> >that if one contributes code one should also at least smoke test > > >> > changes > > >> >somehow. > > >> > > > >> > I don't know if I'm alone with my opinion. I just think that one should > > >> > also do the analysis part and not blindly create and submit patches. > > >> > > >> All of the patches are compile tested. And the individual patches are > > > > > > Compile-testing is not testing. If you are not able to test a commit, > > > you should explain why. > > > > Dude, Julia has been doing semantic patching for years already and > > nobody has raised any concerns so far. There's already an expectation > > that Coccinelle *works* and Julia's sematic patches are sound. > > > > Besides, adding 'const' is something that causes virtually no functional > > changes to the point that build-testing is really all you need. Any > > problems caused by adding 'const' to a definition will be seen by build > > errors or warnings. > > > > Really, just stop with the pointless discussion and go read a bit about > > Coccinelle and what semantic patches are giving you. The work done by > > Julia and her peers are INRIA have measurable benefits. > > > > You're really making a thunderstorm in a glass of water. > > Hmm... I've been using coccinelle in cyclic basis for some time now. > My comment was oversized but I didn't mean it to be impolite or attack > of any kind for that matter. No problem :) Thanks for the feedback. julia -- 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
Re: [PATCH 00/26] constify local structures
On Mon, 12 Sep 2016, Felipe Balbi wrote: > > Hi, > > Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes: > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote: > >> > >> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote: > >> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote: > >> > > Constify local structures. > >> > > > >> > > The semantic patch that makes this change is as follows: > >> > > (http://coccinelle.lip6.fr/) > >> > > >> > Just my two cents but: > >> > > >> > 1. You *can* use a static analysis too to find bugs or other issues. > >> > 2. However, you should manually do the commits and proper commit > >> >messages to subsystems based on your findings. And I generally think > >> >that if one contributes code one should also at least smoke test > >> > changes > >> >somehow. > >> > > >> > I don't know if I'm alone with my opinion. I just think that one should > >> > also do the analysis part and not blindly create and submit patches. > >> > >> All of the patches are compile tested. And the individual patches are > > > > Compile-testing is not testing. If you are not able to test a commit, > > you should explain why. > > Dude, Julia has been doing semantic patching for years already and > nobody has raised any concerns so far. There's already an expectation > that Coccinelle *works* and Julia's sematic patches are sound. > > Besides, adding 'const' is something that causes virtually no functional > changes to the point that build-testing is really all you need. Any > problems caused by adding 'const' to a definition will be seen by build > errors or warnings. > > Really, just stop with the pointless discussion and go read a bit about > Coccinelle and what semantic patches are giving you. The work done by > Julia and her peers are INRIA have measurable benefits. > > You're really making a thunderstorm in a glass of water. Thanks for the defense, but since a lot of these patches torned out to be wrong, due to an incorrect parse by Coccinelle, combined with an unpleasantly lax compiler, Jarkko does have a point that I should have looked at the patches more carefully. In any case, I have written to the maintainers relevant to the patches that turned out to be incorrect. julia -- 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
Re: [PATCH 00/26] constify local structures
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote: > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote: > > > > > > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote: > > > > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote: > > > > Constify local structures. > > > > > > > > The semantic patch that makes this change is as follows: > > > > (http://coccinelle.lip6.fr/) > > > > > > Just my two cents but: > > > > > > 1. You *can* use a static analysis too to find bugs or other issues. > > > 2. However, you should manually do the commits and proper commit > > >messages to subsystems based on your findings. And I generally think > > >that if one contributes code one should also at least smoke test > > > changes > > >somehow. > > > > > > I don't know if I'm alone with my opinion. I just think that one should > > > also do the analysis part and not blindly create and submit patches. > > > > All of the patches are compile tested. And the individual patches are > > Compile-testing is not testing. If you are not able to test a commit, > you should explain why. > > > submitted to the relevant maintainers. The individual commit messages > > give a more detailed explanation of the strategy used to decide that the > > structure was constifiable. It seemed redundant to put that in the cover > > letter, which will not be committed anyway. > > I don't mean to be harsh but I do not care about your thought process > *that much* when I review a commit (sometimes it might make sense to > explain that but it depends on the context). > > I mostly only care why a particular change makes sense for this > particular subsystem. The report given by a static analysis tool can > be a starting point for making a commit but it's not sufficient. > Based on the report you should look subsystems as individuals. OK, thanks for the feedback. julia -- 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
Re: [PATCH 00/26] constify local structures
On Sun, 11 Sep 2016, Jarkko Sakkinen wrote: > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote: > > Constify local structures. > > > > The semantic patch that makes this change is as follows: > > (http://coccinelle.lip6.fr/) > > Just my two cents but: > > 1. You *can* use a static analysis too to find bugs or other issues. > 2. However, you should manually do the commits and proper commit >messages to subsystems based on your findings. And I generally think >that if one contributes code one should also at least smoke test changes >somehow. > > I don't know if I'm alone with my opinion. I just think that one should > also do the analysis part and not blindly create and submit patches. All of the patches are compile tested. And the individual patches are submitted to the relevant maintainers. The individual commit messages give a more detailed explanation of the strategy used to decide that the structure was constifiable. It seemed redundant to put that in the cover letter, which will not be committed anyway. julia > > Anyway, I'll apply the TPM change at some point. As I said they were > for better. Thanks. > > /Jarkko > > > // > > // The first rule ignores some cases that posed problems > > @r disable optional_qualifier@ > > identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer}; > > identifier i != {s5k5baf_cis_rect,smtcfb_fix}; > > position p; > > @@ > > static struct s i@p = { ... }; > > > > @lstruct@ > > identifier r.s; > > @@ > > struct s { ... }; > > > > @used depends on lstruct@ > > identifier r.i; > > @@ > > i > > > > @bad1@ > > expression e; > > identifier r.i; > > assignment operator a; > > @@ > > (<+...i...+>) a e > > > > @bad2@ > > identifier r.i; > > @@ > > &(<+...i...+>) > > > > @bad3@ > > identifier r.i; > > declarer d; > > @@ > > d(...,<+...i...+>,...); > > > > @bad4@ > > identifier r.i; > > type T; > > T[] e; > > identifier f; > > position p; > > @@ > > > > f@p(..., > > ( > > (<+...i...+>) > > & > > e > > ) > > ,...) > > > > @bad4a@ > > identifier r.i; > > type T; > > T *e; > > identifier f; > > position p; > > @@ > > > > f@p(..., > > ( > > (<+...i...+>) > > & > > e > > ) > > ,...) > > > > @ok5@ > > expression *e; > > identifier r.i; > > position p; > > @@ > > e =@p i > > > > @bad5@ > > expression *e; > > identifier r.i; > > position p != ok5.p; > > @@ > > e =@p (<+...i...+>) > > > > @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@ > > identifier s,r.i; > > position r.p; > > @@ > > > > static > > +const > > struct s i@p = { ... }; > > > > @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5 > > disable optional_qualifier@ > > identifier rr.s,r.i; > > @@ > > > > static > > +const > > struct s i; > > // > > > > --- > > > > drivers/acpi/acpi_apd.c |8 +++--- > > drivers/char/tpm/tpm-interface.c | 10 > > drivers/char/tpm/tpm-sysfs.c |2 - > > drivers/cpufreq/intel_pstate.c |8 +++--- > > drivers/infiniband/hw/i40iw/i40iw_uk.c |6 ++--- > > drivers/media/i2c/tvp514x.c |2 - > > drivers/media/pci/ddbridge/ddbridge-core.c | 18 +++ > > drivers/media/pci/ngene/ngene-cards.c| 14 ++-- > > drivers/media/pci/smipcie/smipcie-main.c |8 +++--- > > drivers/misc/sgi-xp/xpc_uv.c |2 - > > drivers/net/arcnet/com20020-pci.c| 10 > > drivers/net/can/c_can/c_can_pci.c|4 +-- > > drivers/net/can/sja1000/plx_pci.c| 20 > > - > > drivers/net/ethernet/mellanox/mlx4/main.c|4 +-- > > drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 - > > drivers/net/ethernet/renesas/sh_eth.c| 14 ++-- > > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |2 - > >
Re: [PATCH 00/26] constify local structures
On Sun, 11 Sep 2016, Joe Perches wrote: > On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote: > > Constify local structures. > > Thanks Julia. > > A few suggestions & questions: > > Perhaps the script should go into scripts/coccinelle/ > so that future cases could be caught by the robot > and commit message referenced by the patch instances. OK. > Can you please compile the files modified using the > appropriate defconfig/allyesconfig and show the I currently send patches for this issue only for files that compile using the x86 allyesconfig. > movement from data to const by using > $ size .new/old > and include that in the changelogs (maybe next time)? OK, thanks for the suggestion. > Is it possible for a rule to trace the instances where > an address of a struct or struct member is taken by > locally defined and declared function call where the > callee does not modify any dereferenced object? > > ie: > > struct foo { > int bar; > char *baz; > }; > > struct foo qux[] = { > { 1, "description 1" }, > { 2, "dewcription 2" }, > [ n, "etc" ]..., > }; > > void message(struct foo *msg) > { > printk("%d %s\n", msg->bar, msg->baz); > } > > where some code uses > > message(qux[index]); > > So could a coccinelle script change: > > struct foo qux[] = { to const struct foo quz[] = { > > and > > void message(struct foo *msg) to void message(const struct foo *msg) Yes, this could be possible too. Thanks for the feedback. julia -- 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