Re: [PATCH]V4L:some v4l drivers have error for video_register_device
Em Thu, 04 Jun 2009 17:20:50 +0800 figo.zhang figo.zh...@kolorific.com escreveu: On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. if (err) is easier for reading, since it is closer to the natural language. Also, as CodingStyle says, C is a Spartan language. So, using just if (err) is better than if (err != 0). Also, since positive values don't indicate errors (on Linux, all errors are negative values), using err != 0 looks wrong. Yet, changing they to err 0 would require a careful review of the returned values by each function. In summary, for newer code, the better is to always use if (err 0), being sure that the called function will return a negative value. However, I don't see much sense on changing the current code. Cheers, 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: [PATCH]V4L:some v4l drivers have error for video_register_device
On Friday 05 June 2009 04:51:36 Figo.zhang wrote: On Thu, 2009-06-04 at 11:27 +0200, Hans Verkuil wrote: On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. To be honest, I think '(err)' is easier to read. Unless there is some new CodingStyle rule I'm not aware of I see no reason for applying these changes. Regards, Hans yes, but i found the the kernel code using the '(err != 0) or (err == 0)' is more popular,in v4l code for example: v4l1-compat.c line 507 v4l2-int-device.c line 52 arv.c line 333 arv.c line 844,856 videobuf-core.c line 529,766,984,1002,1053 videobuf-dma-sg.c line 211,222,248,350,456,671, . so i dont know which style is recommended for kernel code? They are both equally valid. It simply doesn't matter, it is up to the programmer to use whatever he prefers. But a quick grep over the v4l-dvb sources clearly shows a preference for '(err)': grep '(err [!=]= 0)' -rsI .|wc -l 29 grep '(err)' -rsI .|wc -l 331 Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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]V4L:some v4l drivers have error for video_register_device
Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart -- 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]V4L:some v4l drivers have error for video_register_device
On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. -- 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]V4L:some v4l drivers have error for video_register_device
On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. To be honest, I think '(err)' is easier to read. Unless there is some new CodingStyle rule I'm not aware of I see no reason for applying these changes. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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]V4L:some v4l drivers have error for video_register_device
On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. btw, see driver/media/video/ov511.c (line 5853): if (video_register_device(ov-vdev, VFL_TYPE_GRABBER, unit_video[i]) = 0) { break; } it would not return a positive number,i think. pls see my other path: http://www.spinics.net/lists/linux-media/msg06140.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: [PATCH]V4L:some v4l drivers have error for video_register_device
On Thu, 2009-06-04 at 11:27 +0200, Hans Verkuil wrote: On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: Hi, On Thursday 04 June 2009 06:20:07 figo.zhang wrote: The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); What's the difference ? (err != 0) and (err) are identical. Best regards, Laurent Pinchart yes, it is the same, but it is easy for reading. To be honest, I think '(err)' is easier to read. Unless there is some new CodingStyle rule I'm not aware of I see no reason for applying these changes. Regards, Hans yes, but i found the the kernel code using the '(err != 0) or (err == 0)' is more popular,in v4l code for example: v4l1-compat.c line 507 v4l2-int-device.c line 52 arv.c line 333 arv.c line 844,856 videobuf-core.c line 529,766,984,1002,1053 videobuf-dma-sg.c line 211,222,248,350,456,671, . so i dont know which style is recommended for kernel code? Best Regards, Figo.zhang -- 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]V4L:some v4l drivers have error for video_register_device
The function video_register_device() will call the video_register_device_index(). In this function, firtly it will do some argments check , if failed,it will return a negative number such as -EINVAL, and then do cdev_alloc() and device_register(), if success return zero. so video_register_device_index() canot return a a positive number. for example, see the drivers/media/video/stk-webcam.c (line 1325): err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); in my opinion, it will be cleaner to do something like this: err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1); if (err != 0) STK_ERROR(v4l registration failed\n); else STK_INFO(Syntek USB2.0 Camera is now controlling video device /dev/video%d\n, dev-vdev.num); Figo.zhang On Wed, 2009-05-27 at 11:25 +0800, Figo.zhang wrote: For video_register_device(), it return zero means register success.It would be return zero or a negative number (on failure),it would not return a positive number.so it have better to use this to check err: int err = video_register_device(vdev, VFL_TYPE_GRABBER, -1); if (err != 0) { /*err code*/ } Signed-off-by: Figo.zhang figo1...@gmail.com --- Documentation/video4linux/v4l2-framework.txt |2 +- drivers/media/radio/radio-maestro.c |2 +- drivers/media/radio/radio-si470x.c |2 +- drivers/media/video/cafe_ccic.c |2 +- drivers/media/video/cx231xx/cx231xx-video.c |2 +- drivers/media/video/em28xx/em28xx-video.c|2 +- drivers/media/video/et61x251/et61x251_core.c |2 +- drivers/media/video/hdpvr/hdpvr-video.c |2 +- drivers/media/video/ivtv/ivtv-streams.c |2 +- drivers/media/video/sn9c102/sn9c102_core.c |2 +- drivers/media/video/stk-webcam.c |2 +- drivers/media/video/w9968cf.c|2 +- drivers/media/video/zc0301/zc0301_core.c |2 +- drivers/media/video/zr364xx.c|2 +- sound/i2c/other/tea575x-tuner.c |2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt index 854808b..250232a 100644 --- a/Documentation/video4linux/v4l2-framework.txt +++ b/Documentation/video4linux/v4l2-framework.txt @@ -447,7 +447,7 @@ Next you register the video device: this will create the character device for you. err = video_register_device(vdev, VFL_TYPE_GRABBER, -1); - if (err) { + if (err != 0) { video_device_release(vdev); /* or kfree(my_vdev); */ return err; } diff --git a/drivers/media/radio/radio-maestro.c b/drivers/media/radio/radio-maestro.c index 64d737c..b5e93c2 100644 --- a/drivers/media/radio/radio-maestro.c +++ b/drivers/media/radio/radio-maestro.c @@ -379,7 +379,7 @@ static int __devinit maestro_probe(struct pci_dev *pdev, video_set_drvdata(dev-vdev, dev); retval = video_register_device(dev-vdev, VFL_TYPE_RADIO, radio_nr); - if (retval) { + if (retval != 0) { v4l2_err(v4l2_dev, can't register video device!\n); goto errfr1; } diff --git a/drivers/media/radio/radio-si470x.c b/drivers/media/radio/radio-si470x.c index bd945d0..edb520a 100644 --- a/drivers/media/radio/radio-si470x.c +++ b/drivers/media/radio/radio-si470x.c @@ -1740,7 +1740,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf, /* register video device */ retval = video_register_device(radio-videodev, VFL_TYPE_RADIO, radio_nr); - if (retval) { + if (retval != 0) { printk(KERN_WARNING DRIVER_NAME : Could not register video device\n); goto err_all; diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c index c4d181d..fd93698 100644 --- a/drivers/media/video/cafe_ccic.c +++ b/drivers/media/video/cafe_ccic.c @@ -1974,7 +1974,7 @@ static int cafe_pci_probe(struct pci_dev *pdev, /* cam-vdev.debug = V4L2_DEBUG_IOCTL_ARG;*/ cam-vdev.v4l2_dev = cam-v4l2_dev; ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1); - if (ret) + if (ret != 0) goto out_smbus; video_set_drvdata(cam-vdev, cam); diff --git a/drivers/media/video/cx231xx/cx231xx-video.c b/drivers/media/video/cx231xx/cx231xx-video.c index a23ae73..14e5008 100644 --- a/drivers/media/video/cx231xx/cx231xx-video.c +++ b/drivers/media/video/cx231xx/cx231xx-video.c @@ -2382,7 +2382,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev) /*
[PATCH]V4L:some v4l drivers have error for video_register_device
For video_register_device(), it return zero means register success.It would be return zero or a negative number (on failure),it would not return a positive number.so it have better to use this to check err: int err = video_register_device(vdev, VFL_TYPE_GRABBER, -1); if (err != 0) { /*err code*/ } Signed-off-by: Figo.zhang figo1...@gmail.com --- Documentation/video4linux/v4l2-framework.txt |2 +- drivers/media/radio/radio-maestro.c |2 +- drivers/media/radio/radio-si470x.c |2 +- drivers/media/video/cafe_ccic.c |2 +- drivers/media/video/cx231xx/cx231xx-video.c |2 +- drivers/media/video/em28xx/em28xx-video.c|2 +- drivers/media/video/et61x251/et61x251_core.c |2 +- drivers/media/video/hdpvr/hdpvr-video.c |2 +- drivers/media/video/ivtv/ivtv-streams.c |2 +- drivers/media/video/sn9c102/sn9c102_core.c |2 +- drivers/media/video/stk-webcam.c |2 +- drivers/media/video/w9968cf.c|2 +- drivers/media/video/zc0301/zc0301_core.c |2 +- drivers/media/video/zr364xx.c|2 +- sound/i2c/other/tea575x-tuner.c |2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt index 854808b..250232a 100644 --- a/Documentation/video4linux/v4l2-framework.txt +++ b/Documentation/video4linux/v4l2-framework.txt @@ -447,7 +447,7 @@ Next you register the video device: this will create the character device for you. err = video_register_device(vdev, VFL_TYPE_GRABBER, -1); - if (err) { + if (err != 0) { video_device_release(vdev); /* or kfree(my_vdev); */ return err; } diff --git a/drivers/media/radio/radio-maestro.c b/drivers/media/radio/radio-maestro.c index 64d737c..b5e93c2 100644 --- a/drivers/media/radio/radio-maestro.c +++ b/drivers/media/radio/radio-maestro.c @@ -379,7 +379,7 @@ static int __devinit maestro_probe(struct pci_dev *pdev, video_set_drvdata(dev-vdev, dev); retval = video_register_device(dev-vdev, VFL_TYPE_RADIO, radio_nr); - if (retval) { + if (retval != 0) { v4l2_err(v4l2_dev, can't register video device!\n); goto errfr1; } diff --git a/drivers/media/radio/radio-si470x.c b/drivers/media/radio/radio-si470x.c index bd945d0..edb520a 100644 --- a/drivers/media/radio/radio-si470x.c +++ b/drivers/media/radio/radio-si470x.c @@ -1740,7 +1740,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf, /* register video device */ retval = video_register_device(radio-videodev, VFL_TYPE_RADIO, radio_nr); - if (retval) { + if (retval != 0) { printk(KERN_WARNING DRIVER_NAME : Could not register video device\n); goto err_all; diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c index c4d181d..fd93698 100644 --- a/drivers/media/video/cafe_ccic.c +++ b/drivers/media/video/cafe_ccic.c @@ -1974,7 +1974,7 @@ static int cafe_pci_probe(struct pci_dev *pdev, /* cam-vdev.debug = V4L2_DEBUG_IOCTL_ARG;*/ cam-vdev.v4l2_dev = cam-v4l2_dev; ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1); - if (ret) + if (ret != 0) goto out_smbus; video_set_drvdata(cam-vdev, cam); diff --git a/drivers/media/video/cx231xx/cx231xx-video.c b/drivers/media/video/cx231xx/cx231xx-video.c index a23ae73..14e5008 100644 --- a/drivers/media/video/cx231xx/cx231xx-video.c +++ b/drivers/media/video/cx231xx/cx231xx-video.c @@ -2382,7 +2382,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev) /* register v4l2 video video_device */ ret = video_register_device(dev-vdev, VFL_TYPE_GRABBER, video_nr[dev-devno]); - if (ret) { + if (ret != 0) { cx231xx_errdev(unable to register video device (error=%i).\n, ret); return ret; diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c index 882796e..dcc3aca 100644 --- a/drivers/media/video/em28xx/em28xx-video.c +++ b/drivers/media/video/em28xx/em28xx-video.c @@ -2013,7 +2013,7 @@ int em28xx_register_analog_devices(struct em28xx *dev) /* register v4l2 video video_device */ ret = video_register_device(dev-vdev, VFL_TYPE_GRABBER, video_nr[dev-devno]); - if (ret) { + if (ret != 0) { em28xx_errdev(unable to register video device (error=%i).\n, ret); return ret; diff --git a/drivers/media/video/et61x251/et61x251_core.c b/drivers/media/video/et61x251/et61x251_core.c index d1c1e45..8a767e1 100644