into account.
I'm pretty sure it complains about both equally. If you make fix one
warning it will complain about the other. So you just have to pick
which warning to not care about.
regards,
dan carpenter
to be a gcc extension. So, I decided to check upstream
> >
> > No, this is not a gcc extension. It's part of the latest C standard.
>
> Sure? Where the C standard spec states that? I've been seeking for
> such info for a while, as '= {}' is also my personal preference.
>
> I tried to build the Kernel with clang, just to be sure that this
> won't cause issues with the clang support
My test says that clang works with {}.
I support this in Smatch as well.
regards,
dan carpenter
and silence the static checker warning
by not passing the parameter at all.
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c
b/drivers/media/platform/mtk-vpu/mtk_vpu.c
index f8d35e3ac1dc..616f78b24a79 100644
--- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
+++ b/drivers
k can be removed.
The other thing is that if "ret" is an error code here, then we don't
want to do the next call to cam_i2c_write(), so actually let's flip that
test around and return the error. This is more of a theoretical issue
than something which is likely to affect real life.
Signed-off
;
> unsigned int i, k;
> + size_t size;
> +
> + size = num_ref_pics * 4 * 8;
> + memset(vde->iram, 0, size);
I can't get behind the magical size calculation... :(
regards,
dan carpenter
se it to user space. */
1199 v4l2_ctrl_new_std(>ctrls, _ctrl_ops,
regards,
dan carpenter
The > should be >= so we don't read one element beyond the end of the
ca->slot_info[] array. The array is allocated in dvb_ca_en50221_init().
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 131
If info->pdata is NULL then we would oops on the next line. And we can
flip the "ret" test around and give up if a failure has already occured.
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 2a4882cddc51..4ebd00
ULL
> > due
> > to line 26)
> > - On line 28, smatch assumes that vsp1->bru is not NULL
> > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > due
> > to line 28)
> > - On line 31, brx is assigned a possibly NULL value (as there's no
> > information
> > regarding vsp1->brs)
> > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > - On line 36 pipe->brx is dereferenced
> >
> > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > NULL.
> > Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the warning
> > disappear.
> >
I will respond to the other emails in this thread. You guys are
basically spot on. All this analysis is 100% correct. Btw, if you want
to see Smatch's internal state you can do:
#include "/home/whatever/smatch/check_debug.h"
else if (!vsp1->bru->entity.pipe) {
__smatch_implied(>bru->entity);
And it tells you what Smatch thinks it is at that point. The
__smatch_about() output can also be useful.
regards,
dan carpenter
entity can be == to pipe->brx and NULL.
Adding a "(void)vsp1->bru->entity;" on that path will silence the
warning (hopefully).
regards,
dan carpenter
>bru->entity and smatch_extra.c has the
information about if brx is NULL or non-NULL. They don't really share
information very well.
regards,
dan carpenter
bru->entity.pipe)
brx = >bru->entity;
Then Smatch sees that vsp1->bru is dereferenced and marks "brx" as
non-NULL.
regards,
dan carpenter
rmat.pad = pipe->brx->source_pad
>
>
> (1) ensures that brx can't be NULL. (2) is thus always true if pipe->brx is
> NULL. (3) then assigns a non-NULL value to pipe->brx. Smatch should thus
> never
> complain about (4), even if it can't backtrack.
Oh wow... That's a very basic and ancient bug. I've written a fix and
will push it out tomorrow if it passes my testing.
regards,
dan carpenter
On Mon, May 28, 2018 at 01:54:18PM +0300, Laurent Pinchart wrote:
> Hi Dan,
>
> Thank you for your quick reply.
>
> On Monday, 28 May 2018 13:20:49 EEST Dan Carpenter wrote:
> > On Mon, May 28, 2018 at 11:28:41AM +0300, Laurent Pinchart wrote:
> > > and I sti
struct vsp1_entity *released_brx = NULL;
>
Just to be clear. After this patch, Smatch does *NOT* think that
"pipe->brx" is necessarily non-NULL. What this patch does it that
Smatch says "pipe->brx has been modified on every code path since we
checked for NULL, so forget about the earlier check".
regards,
dan carpenter
On Fri, May 25, 2018 at 03:16:21PM +0200, Hans Verkuil wrote:
> On 25/05/18 15:12, Dan Carpenter wrote:
> > In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
> > the problem is that temp_index can be negative. I've made
> > cgf->num_outp
In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
the problem is that temp_index can be negative. I've made
cgf->num_outputs unsigned to fix this issue.
Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
Signed-off-by: Dan Carpenter <da
arlier in a central place. There is no
one who need p->index to be more than INT_MAX.
Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v
If we pick a very large "edid->blocks" value then the "edid->start_block
+ edid->blocks" addition could wrap around.
Fixes: ef834f7836ec ("[media] vivid: add the video capture and output parts")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com&g
() warn:
> > potential spectre issue 'p->ule_next_hdr'
>
> I failed to see what's wrong here, or if this is exploited.
Oh... Huh. This is a bug in smatch. That line looks like:
p->ule_sndu_type = ntohs(*(__be16 *)(p->ule_next_hdr + ((p->ule_dbit ?
2 : 3) * ETH_AL
On Tue, May 15, 2018 at 12:29:10PM -0500, Gustavo A. R. Silva wrote:
>
>
> On 05/15/2018 09:16 AM, Dan Carpenter wrote:
> > > >
> > > > I'm curious about how you finally resolved to handle these issues.
> > > >
> &g
_FUNC macro. This fixes indirect call
> mismatches with Control-Flow Integrity, caused by calling standard
> ioctls using a function pointer that doesn't match the function type.
>
> Signed-off-by: Sami Tolvanen <samitolva...@google.com>
> Signed-off-by: Hans Verkuil <hansv...@cisco.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>
>
Possibly... There was an ancient bug in Smatch's function pointer
handling. I just pushed a fix for it now so the warning is there on
linux-next.
regards,
dan carpenter
bug would exist too... If there were really a race condition like
that then we'd want to fix it instead. In other words, this is not a
real life bug fix.
But it would be fine as a readability or static checker fix so that's
fine.
regards,
dan carpenter
:
351 mutex_unlock(>group->lock);
352
353 kref_put(>group->refcount, rvin_group_release);
There are a bunch of NULL dereferences here...
354 }
regards,
dan carpenter
04 struct v4l2_format *f)
405 {
406 struct fimc_isp *isp = video_drvdata(file);
407
408 __isp_video_try_fmt(isp, >fmt.pix_mp, NULL);
And yet here we are.
409 return 0;
410 }
regards,
dan carpenter
rval = module_if->set(ipipe, to);
> if (rval)
> @@ -1287,7 +1287,9 @@ static int ipipe_s_config(struct v4l2_subdev *sd,
> struct vpfe_ipipe_config *cfg)
> }
> kfree(params);
> }
> + return rval;
Doing a "return 0;" is more readable than "return rval;".
regards,
dan carpenter
gt; > array _format_
> >
> > Notice that given that speculation windows are large, the policy is
> > to kill the speculation on the first load and not worry if it can be
> > completed with a dependent load/store [1].
> >
> > [1] https://marc.info/?l=li
Smatch complains that "venc" could be unintialized. There a couple
error paths where it looks like maybe that could happen. I don't know
if it's really a bug, but it's reasonable to set "venc" to NULL and
silence the warning.
Signed-off-by: Dan Carpenter <dan.carpen...@
We check "lutdpc->dpc_size" in ipipe_validate_lutdpc_params() but if
it's invalid then we would have corrupted memory already when we do
the memcpy() before calling it.
We don't ever check "gamma->tbl_size" but we should since they come from
the user.
Signed-off-by:
o_ipu) {".
It's not hard to invert the varible in this case, because the only thing
we need to change is imx_media_probe() to set:
+ imxmd->ipu_present = true;
regards,
dan carpenter
((status & MIPI_CSIS_INTSRC_ERRORS) || 1) {
^^^
Was this supposed to make it into the published code?
> + for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
> + if (!(status & state->events[i].mask))
> + continue;
> + state->events[i].counter++;
> + }
> + }
> + spin_unlock_irqrestore(>slock, flags);
> +
> + mipi_csis_write(state, MIPI_CSIS_INTSRC, status);
> +
> + return IRQ_HANDLED;
> +}
> +
regards,
dan carpenter
ind upstream endpoint\n");
> + return ret;
> + }
> +
> + mutex_lock(>lock);
> +
> + csi->upstream_ep = upstream_ep;
> + csi->is_csi2 = (upstream_ep.bus_type == V4L2_MBUS_CSI2);
> +
> + mutex_unlock(>lock);
> +
> + return ret;
return 0;
> +}
> +
[ snip ]
> +
> +static int imx7_csi_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
There is no need for this empty (struct platform_driver)->remove()
function. See platform_drv_remove() for how it's called.
This looks nice, though.
regards,
dan carpenter
var16[i] = var8[i];
>
> +#ifdef CONFIG_64BIT
> /* To avoid owerflows when calling the efivar API */
> if (*out_len > ULONG_MAX)
> return -EINVAL;
> +#endif
I should just silence this particular warning in Smatch. I feel like
this is a pretty common thing and the ifdefs aren't very pretty. :(
regards,
dan carpenter
We should check if kzalloc() fails.
Fixes: 8f569c0b4e6b ("media: dvb-core: add helper functions for I2C binding")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index cf747d753a79..787
On Tue, Mar 27, 2018 at 02:00:45PM +0900, Ji-Hun Kim wrote:
>
> Are there any opinions? I'd like to know how this patch is going.
>
Looks good. Thanks!
Greg just hasn't gotten to it yet.
regards,
dan carpenter
array' 14 <= 20
It's always good to simplify the code, but I have a fix for this that I
will publish very soon.
regards,
dan carpenter
credit to:
Reported-by: "Yavuz, Tuba" <t...@ece.ufl.edu>
regards,
dan carpenter
On Tue, Mar 20, 2018 at 02:30:45PM +, Yavuz, Tuba wrote:
> Hello,
>
>
> It looks like there is a double-free on an error path in the zr364xx_probe
> function of the zr364xx
Looks good. Thanks!
regards,
dan carpenter
ackrf_video_release;
then the calls to:
video_unregister_device(>rx_vdev);
and
kfree(dev);
are a double free.
regards,
dan carpenter
On Tue, Mar 20, 2018 at 01:48:17PM +, Yavuz, Tuba wrote:
> Hello,
>
>
> It looks like there is a double-free vulnerability on an
ors all do a return or a goto so "rval"
would be zero here. Then the error path would look like:
err_free_params:
kfree(params);
return rval;
}
regards,
dan carpenter
ain goto free_params.
1298 }
1299 kfree(params);
1300 }
1301 }
1302 error:
1303 return rval;
Change this to:
return 0;
free_params:
kfree(params);
return rval;
1304 }
regards,
dan carpenter
MUCH BETTER!111!Exclamationmark! Thanks! :)
regards,
dan carpenter
F at stream off */
> bool nfb4eof;/* NFB4EOF encountered during streaming */
> struct completion last_eof_comp;
regards,
dan carpenter
There is a double sizeof() typo here so we don't duplicate the struct
properly.
Fixes: be7fd3c3a8c5 ("media: em28xx: Hauppauge DualHD second tuner
functionality")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/d
>> 8) & 0x03);
> >
>
> Won't your analyzer in that case point out that
> "15 >> 8 is zero" again? I may have been underestimating it though
>
It will complain, yes, but it's a pretty common false positive and I
have it in the back of my head to teach the static checker to look for
that situation. Eventually I will get around to it.
regards,
dan carpenter
On Fri, Mar 02, 2018 at 03:20:16PM +0100, jacopo mondi wrote:
> Hi Dan,
>
> On Thu, Mar 01, 2018 at 12:59:54PM +0300, Dan Carpenter wrote:
> > [ I know you're just copying files, but you might have a fix for these
> > since you're looking at the code. - dan ]
>
&
The ">" should be ">=" so that we don't read one element beyond the end
of the array.
Fixes: 8a77009be4be ("media: ov5695: add support for OV5695 sensor")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/i2c/o
(client, VACTIVE_LO,
545 vact & 0xff);
546
547 return ret;
548 }
regards,
dan carpenter
pos = ret & 0x0f;
729 if (pos < 0x0f)
^^
Smatch thinks this implies pos can be 0-14.
730 type |= regs[pos].type.vbi_type;
^
This array only has 5 elements.
731 }
732
733 return type;
734 }
regards,
dan carpenter
roducing shorter aliases is the right thing. But here
it just makes it look like a variable when it's a constant. It's makes
the code slightly less readable.
Just ignore the warning.
regards,
dan carpenter
2c_client *i2c, const
> struct i2c_device_id *id)
> adv7511_audio_init(dev, adv7511);
> return 0;
>
> +err_unregister_packet:
> + i2c_unregister_device(adv7511->i2c_packet);
> err_unregister_cec:
> i2c_unregister_device(adv7511->i2c_cec);
> if (adv7511->cec_clk)
regards,
dan carpenter
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)
drivers/usb/gadget/udc/goku_udc.h:#define INT_EPnNAK(n) (0x00100 < (n))
/* 0 < n < 4 */
regards,
dan carpenter
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
> >
- reissue_mask |= 0x < 4;
+ reissue_mask |= 0x << 4;
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
>
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 0bf031b7e4fa..2a4882cddc51 100644
--- a/drivers/media/i2c/sr030pc30.c
+++ b/drivers/media/i2c/sr030pc30.c
@@ -511,13 +511,16 @@ static int sr030pc30_g
videobuf2-v4l2.c is called by
> vb2_core_dqbuf().
> And that __fill_v4l2_buffer() overwrited the index field: b->index =
> vb->index;
>
> So after the vb2_dqbuf call the buf->index field is correct and bounded.
>
Ah.. I get it. Thanks.
regards,
dan carpenter
On Thu, Jan 25, 2018 at 10:58:45AM +0100, Andrzej Hajda wrote:
> On 23.01.2018 09:32, Dan Carpenter wrote:
> > Hello Andrzej Hajda,
> >
> > The patch 4d0b0ed63660: "[media] s5p-mfc: use MFC_BUF_FLAG_EOS to
> > identify last buffers in decoder capture qu
support")
> > Lets remove this kconfig option.
>
> First of all, I hardly understand how that change is related.
It's pretty obvious if you look at the commit.
-obj-$(CONFIG_VIDEO_ATOMISP_OV8858) += atomisp-ov8858.o
It sounds like you deleted that line by mistake and re-add
On Mon, Jan 22, 2018 at 09:50:04PM +0100, Sylwester Nawrocki wrote:
> On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> > --- a/drivers/media/platform/s3c-camif/camif-capture.c
> > +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> > @@ -1261,11 +1261,11 @@ static void __ca
>Mode ? 1 : 1);
^^^
1818 status += MXL_ControlWrite(fe, EN_CHP_LIN_B, state->Mode ? 0 :
0);
^^^
What do these mean? What are they supposed to do?
1819
regards,
dan carpenter
I couldn't find the check.
659 v4l2_event_queue_fh(>fh, );
660 return 0;
661 default:
662 return -EINVAL;
663 }
664 }
regards,
dan carpenter
e we found the
right format and I've removed the "mf->code = camif_mbus_formats[i];"
because that is a no-op anyway.
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series
camera interface")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
We recently changed this error handling around but missed this error
pointer check. We're testing "priv->vdi_in_ch_n" instead of "ch" so the
error handling can't be triggered.
Fixes: 0b2e9e7947e7 ("media: staging/imx: remove confusing IS_ERR_OR_NULL
usage&qu
The "txbuf" is uninitialized when we call ir_raw_encode_scancode() so
this failure path would lead to a crash.
Fixes: a74b2bff5945 ("media: lirc: do not pass ERR_PTR to kfree")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/rc/lir
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)
> >
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.
regards,
dan carpenter
goto err_free_something;
return 0;
>
> - return ret;
> out_free:
> v4l2_device_unregister_subdev(>sd);
> kfree(dev);
regards,
dan carpenter
I'm really sorry. This warning showed up in my new warnings pile and I
didn't look at the date. :/ Sorry for the noise.
regards,
dan carpenter
Anyway, it's
more clear to just "return 0;"
1306 }
1307 return -1;
^^
-1 is not a proper error code.
1308 }
regards,
dan carpenter
Are you sure you want to hold the lock
this whole time?
368 }
369 }
370
371 out:
372 mutex_unlock(>lock);
373 kfree(txbuf);
^
Can't pass an error pointer to kfree().
374 kfree(raw);
regards,
dan carpenter
Smatch complains that "err" can be uninitialized if we have a zero size
write. The flow analysis is a little complicated so I'm not sure if
that's possible or not, but it's harmless to set this to zero and it
makes the code easier to read.
Signed-off-by: Dan Carpenter <dan.carpen.
On Sat, Dec 02, 2017 at 08:41:48PM +, Jeremy Sowden wrote:
> On 2017-12-02, at 10:35:06 +, Jeremy Sowden wrote:
> > On 2017-12-02, at 13:20:09 +0300, Dan Carpenter wrote:
> > > On Fri, Dec 01, 2017 at 05:19:37PM +, Jeremy Sowden wrote:
> > > &
= DEFAULT_SHADING_INFO, \
> + .grid_info = DEFAULT_GRID_INFO, \
> + .num_invalid_frames = 0 \
> + } \
> +)
We need to get better compile test coverage on this... :/ There are
some others as well.
regards,
dan carpenter
I can't apply this (to today's linux-next) but does this really work:
> +(struct ia_css_3a_grid_info) { \
> + .ae_enable = 0, \
> + .ae_grd_info= (struct ae_public_config_grid_config) { \
> + width = 0, \
> +
return -ENOTSUPP;
2467 } /* switch */
2468
2469 return err;
^^
2470 }
regards,
dan carpenter
On Tue, Nov 28, 2017 at 11:33:37PM +, Jeremy Sowden wrote:
> On 2017-11-28, at 17:15:24 +0300, Dan Carpenter wrote:
> > On Mon, Nov 27, 2017 at 12:44:48PM +, Jeremy Sowden wrote:
> > > The "address" member of struct ia_css_host_data is a
> > > point
carcass it's still stinking up
the living room. We should leave the warning there until it irritates
someone enough to fix it properly.
regards,
dan carpenter
;
>
> if (!err)
> return 0;
>
> and then remove a check above.
>
Argh Success handling. Always do failure handling, never success
handling.
The rest of your comments I agree with, though.
regards,
dan carpenter
support")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c
b/drivers/media/usb/cpia2/cpia2_v4l.c
index 3dedd83f0b19..a1c59f19cf2d 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
tree: git://git.ragnatech.se/linux media-tree
head: bbae615636155fa43a9b0fe0ea31c678984be864
commit: 0985dd306f727df6c0e71cd8a8eda93e8fa5206e [2807/2822] media: imx274:
V4l2 driver for Sony imx274 CMOS sensor
drivers/media/i2c/imx274.c:659 imx274_regmap_util_write_table_8() error:
2df ("[media] media: Add i.MX media core driver")
I normally leave off the Fixes tag when it's not a bugfix. The warning
is, as you mentioned, harmless.
regards,
dan carpenter
timer = 1;
1689
1690 *ber /= timer;
1691 dprintk(verbose, MB86A16_DEBUG, 1, "BER
fine=[0x%02x]", *ber);
1692 } else {
regards,
dan carpenter
We can remove the check for if "state->cec_adap" is NULL. The
cec_allocate_adapter() function never returns NULL and also we verified
that "state->cec_adap" is an error pointer.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/media/i2
lure;
>
> return 0;
> +
> +report_failure:
> + pr_err("dst_gpio_outb ERROR !\n");
> + return -1;
This code is ugly and this patch doesn't improve it; it just shuffles
it around.
regards,
dan carpenter
We free the stk_camera device too early. It's allocate first in probe
and it should be freed last in stk_camera_disconnect().
Reported-by: Andrey Konovalov <andreyk...@google.com>
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
Not tested but these bug reports seem
__func__, rc);
You've been asked several times not to write code like this. You do
it later in the patch series as well.
regards,
dan carpenter
No. Multi-line indents get curly braces for readability.
regards,
dan carpenter
heckpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
regards,
dan carpenter
Looks good.
Reviewed-by: Dan Carpenter <dan.carpen...@oracle.com>
regards,
dan carpenter
been told several times that this stuff doesn't work. Try
applying this patch with `git am` and you'll see why.
regards,
dan carpenter
call
smsusb_term_device(intf); The call tree is:
smsusb_term_device()
-> smscore_unregister_device()
-> smscore_notify_clients()
-> smsdvb_onremove()
-> smsdvb_unregister_client()
-> smsdvb_media_device_unregister()
-> kfree(coredev->media_dev);
regards,
dan carpenter
would be used instead of the failure
> indication "-1"?
>
If you want to, yeah, that would be good.
regards,
dan carpenter
return ret;
> - }
> + if (ret < 0)
> + goto free_snd;
> +
I think the original code is buggy. It should probably call
snd_card_free() if snd_device_new() fails.
regards,
dan carpenter
@@ static int s2250_probe(struct i2c_client *client,
> /* initialize the audio */
> if (write_regs(audio, aud_regs) < 0) {
> dev_err(>dev, "error initializing audio\n");
> - goto fail;
> + goto e_io;
Preserve the error code.
regards,
dan carpenter
<allen.l...@gmail.com>
> ---
^^^
Then under the --- line put:
v4: Update the commit message.
regards,
dan carpenter
On Wed, Sep 13, 2017 at 02:09:25PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais <allen.l...@gmail.com>
This is going through linux-media and maybe they won't insist on some
kind of commit message, but I know Greg does.
regards,
dan carpenter
Also change the subject prefix to: [media] atomisp: so it's:
Subject: [PATCH v2] [media] atomisp: Use ARRAY_SIZE() instead of open coding it
regards,
dan carpenter
h_css.c
> @@ -451,7 +451,7 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
> IA_CSS_FRAME_FORMAT_YUYV
> };
>
> -#define array_length(array) (sizeof(array)/sizeof(array[0]))
> +#define array_length(array) (ARRAY_SIZE(array))
Just get rid of this array_len
), Cherrytrail
> +DEFINES += -DATOMISP_POSTFIX=\"css2401a0_v21\" -DISP2401A0
> +DEFINES += -DSYSTEM_hive_isp_css_2400_system -DISP2401
> -DISP2401_NEW_INPUT_SYSTEM
These defines are a bit ugly. Eventually we will want to get rid of
these.
regards,
dan carpenter
turn_void\n");
> }
>
> -#ifndef ISP2401
> +#if 1 /* was ndef ISP2401 but this function is used by ISP2401 on line 1758
> */
Just delete the #if. (I haven't looked at the code). These comments
should probably be in the changelog. You probably want to break this
patch up into several patches and add a little changelog for each
explaining what's going on.
Extra curly brace. Bad indenting. Add a missing struct member.
Delete references to header file that doesn't exist. Delete defines.
regards,
dan carpenter
1 - 100 of 680 matches
Mail list logo
