Re: [git:v4l-dvb/for_v3.3] [media] cx88-dvb avoid dangling core->gate_ctrl pointer

Re: [git:v4l-dvb/for_v3.3] [media] cx88-dvb avoid dangling core->gate_ctrl pointer

[Jonathan Nieder]

Mauro Carvalho Chehab wrote:

> Subject: [media] cx88-dvb avoid dangling core->gate_ctrl pointer
> Author:  David Fries <david@fries.net>
> Date:    Thu Dec 15 01:59:20 2011 -0300
>
> dvb_register calls videobuf_dvb_register_bus, but if that returns
> a failure the module will be unloaded without clearing the
> value of core->gate_ctrl which will cause an oops in macros
> called from video_open in cx88-video.c
>
> Signed-off-by: David Fries <David@Fries.net>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> Cc: Istvan Varga <istvan_v@mailbox.hu>
> Cc: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.  Here are some patches to stop producing the spurious -ENOMEM
in the first place, and to start checking the return value from
dvb_net_init in other contexts more diligently.  Untested.  Bug
reports, patches in the same vein on top (just try "git grep -Ovi -e
dvb_net_init"), and other thoughts of all kinds welcome.

Jonathan Nieder (9):
  [media] DVB: dvb_net_init: return -errno on error
  [media] videobuf-dvb: avoid spurious ENOMEM when CONFIG_DVB_NET=n
  [media] dvb-bt8xx: use goto based exception handling
  [media] ttusb-budget: use goto for exception handling
  [media] flexcop: handle errors from dvb_net_init
  [media] dvb-bt8xx: handle errors from dvb_net_init
  [media] dm1105: handle errors from dvb_net_init
  [media] dvb-usb: handle errors from dvb_net_init
  [media] firedtv: handle errors from dvb_net_init

 drivers/media/dvb/b2c2/flexcop.c                  |    7 ++-
 drivers/media/dvb/bt8xx/dvb-bt8xx.c               |   65 +++++++++++----------
 drivers/media/dvb/dm1105/dm1105.c                 |    5 +-
 drivers/media/dvb/dvb-core/dvb_net.c              |    4 +-
 drivers/media/dvb/dvb-usb/dvb-usb-dvb.c           |    8 ++-
 drivers/media/dvb/firewire/firedtv-dvb.c          |    5 +-
 drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c |   40 +++++++------
 drivers/media/video/videobuf-dvb.c                |    7 +-
 8 files changed, 82 insertions(+), 59 deletions(-)

[PATCH 1/9] [media] DVB: dvb_net_init: return -errno on error

[Jonathan Nieder]

dvb_net_init unconditionally returns 0.  Callers such as
videobuf_dvb_register_frontend examine dvbnet->dvbdev instead of the
return value to tell whether the operation succeeded.  If it has been
set to a valid pointer, success; if it was left equal to NULL,
failure.

Alas, there is an edge case where that logic does not work as well:
when network support has been compiled out (CONFIG_DVB_NET=n), we want
dvb_net_init and related operations to behave as no-ops and always
succeed, but there is no appropriate value to which to set dvb->dvbdev
to indicate this.

Let dvb_net_init return a meaningful error code, as preparation for
adapting callers to look at that instead.

The only immediate impact of this patch should be to make the few
callers that already check for an error code from dvb_net_init behave
a little more sensibly when it fails.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/dvb-core/dvb_net.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_net.c b/drivers/media/dvb/dvb-core/dvb_net.c
index 93d9869e0f15..8766ce8c354d 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -1510,9 +1510,7 @@ int dvb_net_init (struct dvb_adapter *adap, struct dvb_net *dvbnet,
 	for (i=0; i<DVB_NET_DEVICES_MAX; i++)
 		dvbnet->state[i] = 0;
 
-	dvb_register_device (adap, &dvbnet->dvbdev, &dvbdev_net,
+	return dvb_register_device(adap, &dvbnet->dvbdev, &dvbdev_net,
 			     dvbnet, DVB_DEVICE_NET);
-
-	return 0;
 }
 EXPORT_SYMBOL(dvb_net_init);
-- 
1.7.8.2+next.20111228

[PATCH 2/9] [media] videobuf-dvb: avoid spurious ENOMEM when CONFIG_DVB_NET=n

[Jonathan Nieder]

videobuf_dvb_register_bus relies on dvb_net_init to set dvbnet->dvbdev
on success, but ever since commit fcc8e7d8c0e2 ("dvb_net: Simplify the
code if DVB NET is not defined"), ->dvbdev is left unset when
networking support is disabled.  Therefore in such configurations
videobuf_dvb_register_bus always returns failure, tripping
little-tested error handling paths and preventing the device from
being initialized and used.

Now that dvb_net_init returns a nonzero value on error, we can use
that as a more reliable error indication.  Do so.

Now your card be used with CONFIG_DVB_NET=n, and the kernel will pass
on a more useful error code describing what happened when
CONFIG_DVB_NET=y but dvb_net_init fails due to resource exhaustion.

Reported-by: David Fries <David@Fries.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/videobuf-dvb.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/videobuf-dvb.c b/drivers/media/video/videobuf-dvb.c
index 3de7c7e4402d..59cb54aa2946 100644
--- a/drivers/media/video/videobuf-dvb.c
+++ b/drivers/media/video/videobuf-dvb.c
@@ -226,9 +226,10 @@ static int videobuf_dvb_register_frontend(struct dvb_adapter *adapter,
 	}
 
 	/* register network adapter */
-	dvb_net_init(adapter, &dvb->net, &dvb->demux.dmx);
-	if (dvb->net.dvbdev == NULL) {
-		result = -ENOMEM;
+	result = dvb_net_init(adapter, &dvb->net, &dvb->demux.dmx);
+	if (result < 0) {
+		printk(KERN_WARNING "%s: dvb_net_init failed (errno = %d)\n",
+		       dvb->name, result);
 		goto fail_fe_conn;
 	}
 	return 0;
-- 
1.7.8.2+next.20111228

[PATCH 3/9] [media] dvb-bt8xx: use goto based exception handling

[Jonathan Nieder]

Repeating the same cleanup code in each error handling path makes life
unnecessarily difficult for reviewers, who much check each instance of
the same copy+pasted code separately.  A "goto" to the end of the
function is more maintainable and conveys the intent more clearly.

While we're touching this code, also lift some assignments from "if"
conditionals for simplicity.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/bt8xx/dvb-bt8xx.c |   57 ++++++++++++++++------------------
 1 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/drivers/media/dvb/bt8xx/dvb-bt8xx.c b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
index 521d69104982..6aa3b486e865 100644
--- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
+++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
@@ -741,57 +741,42 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 	card->demux.stop_feed = dvb_bt8xx_stop_feed;
 	card->demux.write_to_decoder = NULL;
 
-	if ((result = dvb_dmx_init(&card->demux)) < 0) {
+	result = dvb_dmx_init(&card->demux);
+	if (result < 0) {
 		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
-
-		dvb_unregister_adapter(&card->dvb_adapter);
-		return result;
+		goto err_unregister_adaptor;
 	}
 
 	card->dmxdev.filternum = 256;
 	card->dmxdev.demux = &card->demux.dmx;
 	card->dmxdev.capabilities = 0;
 
-	if ((result = dvb_dmxdev_init(&card->dmxdev, &card->dvb_adapter)) < 0) {
+	result = dvb_dmxdev_init(&card->dmxdev, &card->dvb_adapter);
+	if (result < 0) {
 		printk("dvb_bt8xx: dvb_dmxdev_init failed (errno = %d)\n", result);
-
-		dvb_dmx_release(&card->demux);
-		dvb_unregister_adapter(&card->dvb_adapter);
-		return result;
+		goto err_dmx_release;
 	}
 
 	card->fe_hw.source = DMX_FRONTEND_0;
 
-	if ((result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_hw)) < 0) {
+	result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_hw);
+	if (result < 0) {
 		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
-
-		dvb_dmxdev_release(&card->dmxdev);
-		dvb_dmx_release(&card->demux);
-		dvb_unregister_adapter(&card->dvb_adapter);
-		return result;
+		goto err_dmxdev_release;
 	}
 
 	card->fe_mem.source = DMX_MEMORY_FE;
 
-	if ((result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_mem)) < 0) {
+	result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_mem);
+	if (result < 0) {
 		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
-
-		card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_hw);
-		dvb_dmxdev_release(&card->dmxdev);
-		dvb_dmx_release(&card->demux);
-		dvb_unregister_adapter(&card->dvb_adapter);
-		return result;
+		goto err_remove_hw_frontend;
 	}
 
-	if ((result = card->demux.dmx.connect_frontend(&card->demux.dmx, &card->fe_hw)) < 0) {
+	result = card->demux.dmx.connect_frontend(&card->demux.dmx, &card->fe_hw);
+	if (result < 0) {
 		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
-
-		card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_mem);
-		card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_hw);
-		dvb_dmxdev_release(&card->dmxdev);
-		dvb_dmx_release(&card->demux);
-		dvb_unregister_adapter(&card->dvb_adapter);
-		return result;
+		goto err_remove_mem_frontend;
 	}
 
 	dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
@@ -801,6 +786,18 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 	frontend_init(card, type);
 
 	return 0;
+
+err_remove_mem_frontend:
+	card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_mem);
+err_remove_hw_frontend:
+	card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_hw);
+err_dmxdev_release:
+	dvb_dmxdev_release(&card->dmxdev);
+err_dmx_release:
+	dvb_dmx_release(&card->demux);
+err_unregister_adaptor:
+	dvb_unregister_adapter(&card->dvb_adapter);
+	return result;
 }
 
 static int __devinit dvb_bt8xx_probe(struct bttv_sub_device *sub)
-- 
1.7.8.2+next.20111228

[PATCH 4/9] [media] ttusb-budget: use goto for exception handling

[Jonathan Nieder]

Avoid some repetition by adopting the usual "goto err" idiom for error
handling.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c |   40 +++++++++++---------
 1 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c b/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
index 420bb42d5233..b0f90b7f0eb1 100644
--- a/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
@@ -1694,10 +1694,8 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	ttusb->i2c_adap.dev.parent	  = &udev->dev;
 
 	result = i2c_add_adapter(&ttusb->i2c_adap);
-	if (result) {
-		dvb_unregister_adapter (&ttusb->adapter);
-		return result;
-	}
+	if (result)
+		goto err_unregister_adapter;
 
 	memset(&ttusb->dvb_demux, 0, sizeof(ttusb->dvb_demux));
 
@@ -1714,33 +1712,29 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	ttusb->dvb_demux.stop_feed = ttusb_stop_feed;
 	ttusb->dvb_demux.write_to_decoder = NULL;
 
-	if ((result = dvb_dmx_init(&ttusb->dvb_demux)) < 0) {
+	result = dvb_dmx_init(&ttusb->dvb_demux);
+	if (result < 0) {
 		printk("ttusb_dvb: dvb_dmx_init failed (errno = %d)\n", result);
-		i2c_del_adapter(&ttusb->i2c_adap);
-		dvb_unregister_adapter (&ttusb->adapter);
-		return -ENODEV;
+		result = -ENODEV;
+		goto err_i2c_del_adapter;
 	}
 //FIXME dmxdev (nur WAS?)
 	ttusb->dmxdev.filternum = ttusb->dvb_demux.filternum;
 	ttusb->dmxdev.demux = &ttusb->dvb_demux.dmx;
 	ttusb->dmxdev.capabilities = 0;
 
-	if ((result = dvb_dmxdev_init(&ttusb->dmxdev, &ttusb->adapter)) < 0) {
+	result = dvb_dmxdev_init(&ttusb->dmxdev, &ttusb->adapter);
+	if (result < 0) {
 		printk("ttusb_dvb: dvb_dmxdev_init failed (errno = %d)\n",
 		       result);
-		dvb_dmx_release(&ttusb->dvb_demux);
-		i2c_del_adapter(&ttusb->i2c_adap);
-		dvb_unregister_adapter (&ttusb->adapter);
-		return -ENODEV;
+		result = -ENODEV;
+		goto err_release_dmx;
 	}
 
 	if (dvb_net_init(&ttusb->adapter, &ttusb->dvbnet, &ttusb->dvb_demux.dmx)) {
 		printk("ttusb_dvb: dvb_net_init failed!\n");
-		dvb_dmxdev_release(&ttusb->dmxdev);
-		dvb_dmx_release(&ttusb->dvb_demux);
-		i2c_del_adapter(&ttusb->i2c_adap);
-		dvb_unregister_adapter (&ttusb->adapter);
-		return -ENODEV;
+		result = -ENODEV;
+		goto err_release_dmxdev;
 	}
 
 	usb_set_intfdata(intf, (void *) ttusb);
@@ -1748,6 +1742,16 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	frontend_init(ttusb);
 
 	return 0;
+
+err_release_dmxdev:
+	dvb_dmxdev_release(&ttusb->dmxdev);
+err_release_dmx:
+	dvb_dmx_release(&ttusb->dvb_demux);
+err_i2c_del_adapter:
+	i2c_del_adapter(&ttusb->i2c_adap);
+err_unregister_adapter:
+	dvb_unregister_adapter (&ttusb->adapter);
+	return result;
 }
 
 static void ttusb_disconnect(struct usb_interface *intf)
-- 
1.7.8.2+next.20111228

[PATCH 5/9] [media] flexcop: handle errors from dvb_net_init

[Jonathan Nieder]

Bail out if dvb_net_init encounters an error (for example an
out-of-memory condition), now that it reports them.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/b2c2/flexcop.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/b2c2/flexcop.c b/drivers/media/dvb/b2c2/flexcop.c
index 2df1b0214dcd..ed3f42776fee 100644
--- a/drivers/media/dvb/b2c2/flexcop.c
+++ b/drivers/media/dvb/b2c2/flexcop.c
@@ -117,11 +117,16 @@ static int flexcop_dvb_init(struct flexcop_device *fc)
 		goto err_connect_frontend;
 	}
 
-	dvb_net_init(&fc->dvb_adapter, &fc->dvbnet, &fc->demux.dmx);
+	if ((ret = dvb_net_init(&fc->dvb_adapter, &fc->dvbnet, &fc->demux.dmx)) < 0) {
+		err("dvb_net_init failed: error %d", ret);
+		goto err_net;
+	}
 
 	fc->init_state |= FC_STATE_DVB_INIT;
 	return 0;
 
+err_net:
+	fc->demux.dmx.disconnect_frontend(&fc->demux.dmx);
 err_connect_frontend:
 	fc->demux.dmx.remove_frontend(&fc->demux.dmx, &fc->mem_frontend);
 err_dmx_add_mem_frontend:
-- 
1.7.8.2+next.20111228

[PATCH 6/9] [media] dvb-bt8xx: handle errors from dvb_net_init

[Jonathan Nieder]

Clean up and error out if dvb_net_init fails (for example when
running out of memory).

>From an audit of dvb_net_init callers, now that dvb_net_init
has learned to return a nonzero value from time to time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/bt8xx/dvb-bt8xx.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/bt8xx/dvb-bt8xx.c b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
index 6aa3b486e865..94e01b47784f 100644
--- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
+++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
@@ -779,7 +779,11 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 		goto err_remove_mem_frontend;
 	}
 
-	dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
+	result = dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
+	if (result < 0) {
+		printk("dvb_bt8xx: dvb_net_init failed (errno = %d)\n", result);
+		goto err_disconnect_frontend;
+	}
 
 	tasklet_init(&card->bt->tasklet, dvb_bt8xx_task, (unsigned long) card);
 
@@ -787,6 +791,8 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 
 	return 0;
 
+err_disconnect_frontend:
+	card->demux.dmx.disconnect_frontend(&card->demux.dmx);
 err_remove_mem_frontend:
 	card->demux.dmx.remove_frontend(&card->demux.dmx, &card->fe_mem);
 err_remove_hw_frontend:
-- 
1.7.8.2+next.20111228

[PATCH 7/9] [media] dm1105: handle errors from dvb_net_init

[Jonathan Nieder]

Clean up and error out if dvb_net_init fails (for example due to
ENOMEM).  This involves moving the dvb_net_init call to before
frontend_init to make cleaning up a little easier.

>From an audit of dvb_net_init callers, now that dvb_net_init lets
callers know about errors.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/dm1105/dm1105.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/dm1105/dm1105.c b/drivers/media/dvb/dm1105/dm1105.c
index 55e6533f15e9..70e040b999e7 100644
--- a/drivers/media/dvb/dm1105/dm1105.c
+++ b/drivers/media/dvb/dm1105/dm1105.c
@@ -1115,11 +1115,14 @@ static int __devinit dm1105_probe(struct pci_dev *pdev,
 	if (ret < 0)
 		goto err_remove_mem_frontend;
 
+	ret = dvb_net_init(dvb_adapter, &dev->dvbnet, dmx);
+	if (ret < 0)
+		goto err_disconnect_frontend;
+
 	ret = frontend_init(dev);
 	if (ret < 0)
 		goto err_disconnect_frontend;
 
-	dvb_net_init(dvb_adapter, &dev->dvbnet, dmx);
 	dm1105_ir_init(dev);
 
 	INIT_WORK(&dev->work, dm1105_dmx_buffer);
-- 
1.7.8.2+next.20111228

[PATCH 8/9] [media] dvb-usb: handle errors from dvb_net_init

[Jonathan Nieder]

>From an audit of dvb_net_init callers, now that that function
returns -errno on error.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/dvb-usb/dvb-usb-dvb.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
index ba4a7517354f..ddf282f355b3 100644
--- a/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/dvb/dvb-usb/dvb-usb-dvb.c
@@ -141,11 +141,17 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums)
 		goto err_dmx_dev;
 	}
 
-	dvb_net_init(&adap->dvb_adap, &adap->dvb_net, &adap->demux.dmx);
+	if ((ret = dvb_net_init(&adap->dvb_adap, &adap->dvb_net,
+						&adap->demux.dmx)) < 0) {
+		err("dvb_net_init failed: error %d",ret);
+		goto err_net_init;
+	}
 
 	adap->state |= DVB_USB_ADAP_STATE_DVB;
 	return 0;
 
+err_net_init:
+	dvb_dmxdev_release(&adap->dmxdev);
 err_dmx_dev:
 	dvb_dmx_release(&adap->demux);
 err_dmx:
-- 
1.7.8.2+next.20111228

[PATCH 9/9] [media] firedtv: handle errors from dvb_net_init

[Jonathan Nieder]

It is not common for dvb_net_init to fail, but after the patch
"dvb_net_init: return -errno on error" it can fail due to running out
of memory.  Handle this.

>From an audit of dvb_net_init callers.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end of the series, though it would have been nice to
also check the error handling in

 dvb/mantis/mantis_dvb.c
 dvb/ngene/ngene-core.c (which looks a little strange)
 dvb/pluto2/pluto2.c
 dvb/pt1/pt1.c
 dvb/ttpci/av7110.c 
 dvb/ttpci/budget-core.c
 dvb/ttusb-dec/ttusb_dec.c
 video/au0828/au0828-dvb.c
 video/cx18/cx18-dvb.c
 video/cx231xx/cx231xx-dvb.c
 video/em28xx/em28xx-dvb.c
 video/pvrusb2/pvrusb2-dvb.c
 video/saa7164/saa7164-dvb.c

Hopefully this gives the idea, anyway.  Patch 2 is the important one,
and the patches after that are just toys to show off patch 1.

Warning: the patches are _completely_ _untested_.  Test results
(perhaps from provoking artificial failures in dvb_net_init), just
like other comments, would be very welcome.

'night,
Jonathan

 drivers/media/dvb/firewire/firedtv-dvb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/firewire/firedtv-dvb.c b/drivers/media/dvb/firewire/firedtv-dvb.c
index fd8bbbfa5c59..eb7496eab130 100644
--- a/drivers/media/dvb/firewire/firedtv-dvb.c
+++ b/drivers/media/dvb/firewire/firedtv-dvb.c
@@ -203,7 +203,9 @@ int fdtv_dvb_register(struct firedtv *fdtv, const char *name)
 	if (err)
 		goto fail_rem_frontend;
 
-	dvb_net_init(&fdtv->adapter, &fdtv->dvbnet, &fdtv->demux.dmx);
+	err = dvb_net_init(&fdtv->adapter, &fdtv->dvbnet, &fdtv->demux.dmx);
+	if (err)
+		goto fail_disconnect_frontend;
 
 	fdtv_frontend_init(fdtv, name);
 	err = dvb_register_frontend(&fdtv->adapter, &fdtv->fe);
@@ -218,6 +220,7 @@ int fdtv_dvb_register(struct firedtv *fdtv, const char *name)
 
 fail_net_release:
 	dvb_net_release(&fdtv->dvbnet);
+fail_disconnect_frontend:
 	fdtv->demux.dmx.close(&fdtv->demux.dmx);
 fail_rem_frontend:
 	fdtv->demux.dmx.remove_frontend(&fdtv->demux.dmx, &fdtv->frontend);
-- 
1.7.8.2+next.20111228

Re: [PATCH 9/9] [media] firedtv: handle errors from dvb_net_init

[Stefan Richter]

On Dec 31 Jonathan Nieder wrote:
> It is not common for dvb_net_init to fail, but after the patch
> "dvb_net_init: return -errno on error" it can fail due to running out
> of memory.  Handle this.
> From an audit of dvb_net_init callers.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Reviewed-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

[...]
> --- a/drivers/media/dvb/firewire/firedtv-dvb.c
> +++ b/drivers/media/dvb/firewire/firedtv-dvb.c
> @@ -203,7 +203,9 @@ int fdtv_dvb_register(struct firedtv *fdtv, const
> char *name) if (err)
>  		goto fail_rem_frontend;
>  
> -	dvb_net_init(&fdtv->adapter, &fdtv->dvbnet, &fdtv->demux.dmx);
> +	err = dvb_net_init(&fdtv->adapter, &fdtv->dvbnet,
> &fdtv->demux.dmx);
> +	if (err)
> +		goto fail_disconnect_frontend;
>  
>  	fdtv_frontend_init(fdtv, name);
>  	err = dvb_register_frontend(&fdtv->adapter, &fdtv->fe);
> @@ -218,6 +220,7 @@ int fdtv_dvb_register(struct firedtv *fdtv, const
> char *name) 
>  fail_net_release:
>  	dvb_net_release(&fdtv->dvbnet);
> +fail_disconnect_frontend:
>  	fdtv->demux.dmx.close(&fdtv->demux.dmx);
>  fail_rem_frontend:
>  	fdtv->demux.dmx.remove_frontend(&fdtv->demux.dmx,
> &fdtv->frontend);

-- 
Stefan Richter
-=====-==-== ==-- =====
http://arcgraph.de/sr/

Re: [PATCH 1/9] [media] DVB: dvb_net_init: return -errno on error

[Darron Broad]

Hi

On Sat, December 31, 2011 11:54, Jonathan Nieder wrote:
> dvb_net_init unconditionally returns 0.  Callers such as
> videobuf_dvb_register_frontend examine dvbnet->dvbdev instead of the
> return value to tell whether the operation succeeded.  If it has been
> set to a valid pointer, success; if it was left equal to NULL,
> failure.

I noticed this when testing the MFE patch set a few years ago
now and as you have seen I tested for NULL elsewhere more as
a reminder than any thing else.

I made no changes either as you can also see since it was beyond
the scope of the MFE patches at the time. I do remember this
and it pops into my mind once in a while and now it can now be
cast aside forever, thanks.

> Alas, there is an edge case where that logic does not work as well:
> when network support has been compiled out (CONFIG_DVB_NET=n), we want
> dvb_net_init and related operations to behave as no-ops and always
> succeed, but there is no appropriate value to which to set dvb->dvbdev
> to indicate this.

I suspect this is the only case where the MFE patches do not
properly check every potential fault with attachment as I cannot
remember any other function being as the NET attachment which
may have actually been a void function when I last visited it
but that's a bit vague and the return 0 suggests not.

> Let dvb_net_init return a meaningful error code, as preparation for
> adapting callers to look at that instead.
>
> The only immediate impact of this patch should be to make the few
> callers that already check for an error code from dvb_net_init behave
> a little more sensibly when it fails.

Cheers, thanks for your efforts.
bye

> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  drivers/media/dvb/dvb-core/dvb_net.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
> b/drivers/media/dvb/dvb-core/dvb_net.c
> index 93d9869e0f15..8766ce8c354d 100644
> --- a/drivers/media/dvb/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb/dvb-core/dvb_net.c
> @@ -1510,9 +1510,7 @@ int dvb_net_init (struct dvb_adapter *adap, struct
> dvb_net *dvbnet,
>  	for (i=0; i<DVB_NET_DEVICES_MAX; i++)
>  		dvbnet->state[i] = 0;
>
> -	dvb_register_device (adap, &dvbnet->dvbdev, &dvbdev_net,
> +	return dvb_register_device(adap, &dvbnet->dvbdev, &dvbdev_net,
>  			     dvbnet, DVB_DEVICE_NET);
> -
> -	return 0;
>  }
>  EXPORT_SYMBOL(dvb_net_init);


-- 

 // /
{:)==={ Darron Broad <darron@kewl.org>
 \\ \

Re: [PATCH 9/9] [media] firedtv: handle errors from dvb_net_init

[Mauro Carvalho Chehab]

On 31-12-2011 10:19, Jonathan Nieder wrote:
> It is not common for dvb_net_init to fail, but after the patch
> "dvb_net_init: return -errno on error" it can fail due to running out
> of memory.  Handle this.
> 
>>From an audit of dvb_net_init callers.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> That's the end of the series, though it would have been nice to
> also check the error handling in
> 
>  dvb/mantis/mantis_dvb.c
>  dvb/ngene/ngene-core.c (which looks a little strange)
>  dvb/pluto2/pluto2.c
>  dvb/pt1/pt1.c
>  dvb/ttpci/av7110.c 
>  dvb/ttpci/budget-core.c
>  dvb/ttusb-dec/ttusb_dec.c
>  video/au0828/au0828-dvb.c
>  video/cx18/cx18-dvb.c
>  video/cx231xx/cx231xx-dvb.c
>  video/em28xx/em28xx-dvb.c
>  video/pvrusb2/pvrusb2-dvb.c
>  video/saa7164/saa7164-dvb.c

It would be good if you could take a look on them and send us patches
for them if needed ;)

> 
> Hopefully this gives the idea, anyway.  Patch 2 is the important one,
> and the patches after that are just toys to show off patch 1.
> 
> Warning: the patches are _completely_ _untested_.  Test results
> (perhaps from provoking artificial failures in dvb_net_init), just
> like other comments, would be very welcome.
> 
> 'night,
> Jonathan
> 
>  drivers/media/dvb/firewire/firedtv-dvb.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/dvb/firewire/firedtv-dvb.c b/drivers/media/dvb/firewire/firedtv-dvb.c
> index fd8bbbfa5c59..eb7496eab130 100644
> --- a/drivers/media/dvb/firewire/firedtv-dvb.c
> +++ b/drivers/media/dvb/firewire/firedtv-dvb.c
> @@ -203,7 +203,9 @@ int fdtv_dvb_register(struct firedtv *fdtv, const char *name)
>  	if (err)
>  		goto fail_rem_frontend;
>  
> -	dvb_net_init(&fdtv->adapter, &fdtv->dvbnet, &fdtv->demux.dmx);
> +	err = dvb_net_init(&fdtv->adapter, &fdtv->dvbnet, &fdtv->demux.dmx);
> +	if (err)
> +		goto fail_disconnect_frontend;
>  
>  	fdtv_frontend_init(fdtv, name);
>  	err = dvb_register_frontend(&fdtv->adapter, &fdtv->fe);
> @@ -218,6 +220,7 @@ int fdtv_dvb_register(struct firedtv *fdtv, const char *name)
>  
>  fail_net_release:
>  	dvb_net_release(&fdtv->dvbnet);
> +fail_disconnect_frontend:
>  	fdtv->demux.dmx.close(&fdtv->demux.dmx);
>  fail_rem_frontend:
>  	fdtv->demux.dmx.remove_frontend(&fdtv->demux.dmx, &fdtv->frontend);