Re: [PATCH]V4L:some v4l drivers have error for video_register_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Thu, 04 Jun 2009 17:20:50 +0800
"figo.zhang" <figo.zhang@xxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux