lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b357f49c-0ce6-18f5-9f3c-fdead5e4c838@xs4all.nl>
Date:   Tue, 28 Jul 2020 17:05:47 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Dhiraj Sharma <dhiraj.sharma0024@...il.com>,
        gregkh@...uxfoundation.org, stern@...land.harvard.edu,
        arnd@...db.de, jrdr.linux@...il.com
Cc:     linux-media@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: usbvision: fixed coding style

Hi Dhiraj,

On 28/07/2020 16:30, Dhiraj Sharma wrote:
> As per eudyptula challenge task 10 I had to fix coding styles. Thus I
> used checkpatch.pl script and fixed a chunk of warnings and few errors.

As both drivers/staging/media/usbvision/Kconfig and .../TODO say, this
driver is deprecated and will be removed by the end of this year.

So don't bother with this driver.

Regards,

	Hans

> 
> Signed-off-by: Dhiraj Sharma <dhiraj.sharma0024@...il.com>
> ---
>  .../staging/media/usbvision/usbvision-video.c | 91 +++++++++++--------
>  1 file changed, 52 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/staging/media/usbvision/usbvision-video.c b/drivers/staging/media/usbvision/usbvision-video.c
> index 3ea25fdcf767..8b68e99a2813 100644
> --- a/drivers/staging/media/usbvision/usbvision-video.c
> +++ b/drivers/staging/media/usbvision/usbvision-video.c
> @@ -67,8 +67,8 @@
>  #ifdef USBVISION_DEBUG
>  	#define PDEBUG(level, fmt, args...) { \
>  		if (video_debug & (level)) \
> -			printk(KERN_INFO KBUILD_MODNAME ":[%s:%d] " fmt, \
> -				__func__, __LINE__ , ## args); \
> +			pr_debug(KBUILD_MODNAME ":[%s:%d] " fmt, \
> +				__func__, __LINE__, ## args); \
>  	}
>  #else
>  	#define PDEBUG(level, fmt, args...) do {} while (0)
> @@ -79,8 +79,8 @@
>  #define DBG_MMAP	(1 << 3)
> 
>  /* String operations */
> -#define rmspace(str)	while (*str == ' ') str++;
> -#define goto2next(str)	while (*str != ' ') str++; while (*str == ' ') str++;
> +#define rmspace(str)	do { str++; } while (*str == ' ')
> +#define goto2next(str)	do { str++; } while (*str != ' ' || *str == ' ')
> 
> 
>  /* sequential number of usbvision device */
> @@ -145,27 +145,29 @@ MODULE_ALIAS(DRIVER_ALIAS);
>  static inline struct usb_usbvision *cd_to_usbvision(struct device *cd)
>  {
>  	struct video_device *vdev = to_video_device(cd);
> +
>  	return video_get_drvdata(vdev);
>  }
> 
> -static ssize_t show_version(struct device *cd,
> +static ssize_t version_show(struct device *cd,
>  			    struct device_attribute *attr, char *buf)
>  {
>  	return sprintf(buf, "%s\n", USBVISION_VERSION_STRING);
>  }
> -static DEVICE_ATTR(version, S_IRUGO, show_version, NULL);
> +static DEVICE_ATTR_RO(version, 0444, version_show, NULL);
> 
> -static ssize_t show_model(struct device *cd,
> +static ssize_t model_show(struct device *cd,
>  			  struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
>  	struct usb_usbvision *usbvision = video_get_drvdata(vdev);
> +
>  	return sprintf(buf, "%s\n",
>  		       usbvision_device_data[usbvision->dev_model].model_string);
>  }
> -static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> +static DEVICE_ATTR_RO(model, 0444, model_show, NULL);
> 
> -static ssize_t show_hue(struct device *cd,
> +static ssize_t hue_show(struct device *cd,
>  			struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
> @@ -175,9 +177,9 @@ static ssize_t show_hue(struct device *cd,
> 
>  	return sprintf(buf, "%d\n", val);
>  }
> -static DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL);
> +static DEVICE_ATTR_RO(hue, 0444, hue_show, NULL);
> 
> -static ssize_t show_contrast(struct device *cd,
> +static ssize_t contrast_show(struct device *cd,
>  			     struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
> @@ -187,9 +189,9 @@ static ssize_t show_contrast(struct device *cd,
> 
>  	return sprintf(buf, "%d\n", val);
>  }
> -static DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL);
> +static DEVICE_ATTR_RO(contrast, 0444, contrast_show, NULL);
> 
> -static ssize_t show_brightness(struct device *cd,
> +static ssize_t brightness_show(struct device *cd,
>  			       struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
> @@ -199,9 +201,9 @@ static ssize_t show_brightness(struct device *cd,
> 
>  	return sprintf(buf, "%d\n", val);
>  }
> -static DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL);
> +static DEVICE_ATTR_RO(brightness, 0444, brightness_show, NULL);
> 
> -static ssize_t show_saturation(struct device *cd,
> +static ssize_t saturation_show(struct device *cd,
>  			       struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
> @@ -211,36 +213,39 @@ static ssize_t show_saturation(struct device *cd,
> 
>  	return sprintf(buf, "%d\n", val);
>  }
> -static DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL);
> +static DEVICE_ATTR_RO(saturation, 0444, saturation_show, NULL);
> 
> -static ssize_t show_streaming(struct device *cd,
> +static ssize_t streaming_show(struct device *cd,
>  			      struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
>  	struct usb_usbvision *usbvision = video_get_drvdata(vdev);
> +
>  	return sprintf(buf, "%s\n",
>  		       YES_NO(usbvision->streaming == stream_on ? 1 : 0));
>  }
> -static DEVICE_ATTR(streaming, S_IRUGO, show_streaming, NULL);
> +static DEVICE_ATTR_RO(streaming, 0444, streaming_show, NULL);
> 
> -static ssize_t show_compression(struct device *cd,
> +static ssize_t compression_show(struct device *cd,
>  				struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
>  	struct usb_usbvision *usbvision = video_get_drvdata(vdev);
> +
>  	return sprintf(buf, "%s\n",
>  		       YES_NO(usbvision->isoc_mode == ISOC_MODE_COMPRESS));
>  }
> -static DEVICE_ATTR(compression, S_IRUGO, show_compression, NULL);
> +static DEVICE_ATTR_RO(compression, 0444, compression_show, NULL);
> 
>  static ssize_t show_device_bridge(struct device *cd,
>  				  struct device_attribute *attr, char *buf)
>  {
>  	struct video_device *vdev = to_video_device(cd);
>  	struct usb_usbvision *usbvision = video_get_drvdata(vdev);
> +
>  	return sprintf(buf, "%d\n", usbvision->bridge_type);
>  }
> -static DEVICE_ATTR(bridge, S_IRUGO, show_device_bridge, NULL);
> +static DEVICE_ATTR_RO(bridge, 0444, show_device_bridge, NULL);
> 
>  static void usbvision_create_sysfs(struct video_device *vdev)
>  {
> @@ -329,7 +334,8 @@ static int usbvision_v4l2_open(struct file *file)
>  		err_code = usbvision_scratch_alloc(usbvision);
>  		if (isoc_mode == ISOC_MODE_COMPRESS) {
>  			/* Allocate intermediate decompression buffers
> -			   only if needed */
> +			 * only if needed
> +			 */
>  			err_code = usbvision_decompress_alloc(usbvision);
>  		}
>  		if (err_code) {
> @@ -344,6 +350,7 @@ static int usbvision_v4l2_open(struct file *file)
>  		/* Send init sequence only once, it's large! */
>  		if (!usbvision->initialized) {
>  			int setup_ok = 0;
> +
>  			setup_ok = usbvision_setup(usbvision, isoc_mode);
>  			if (setup_ok)
>  				usbvision->initialized = 1;
> @@ -400,7 +407,7 @@ static int usbvision_v4l2_close(struct file *file)
>  	mutex_unlock(&usbvision->v4l2_lock);
> 
>  	if (r) {
> -		printk(KERN_INFO "%s: Final disconnect\n", __func__);
> +		pr_debug("%s: Final disconnect\n", __func__);
>  		usbvision_release(usbvision);
>  		return 0;
>  	}
> @@ -490,7 +497,8 @@ static int vidioc_enum_input(struct file *file, void *priv,
>  		chan = vi->index + 1; /* skip Television string*/
> 
>  	/* Determine the requested input characteristics
> -	   specific for each usbvision card model */
> +	 * specific for each usbvision card model
> +	 */
>  	switch (chan) {
>  	case 0:
>  		if (usbvision_device_data[usbvision->dev_model].video_channels == 4) {
> @@ -649,7 +657,8 @@ static int vidioc_reqbufs(struct file *file,
>  	RESTRICT_TO_RANGE(vr->count, 1, USBVISION_NUMFRAMES);
> 
>  	/* Check input validity:
> -	   the user must do a VIDEO CAPTURE and MMAP method. */
> +	 * the user must do a VIDEO CAPTURE and MMAP method.
> +	 */
>  	if (vr->memory != V4L2_MEMORY_MMAP)
>  		return -EINVAL;
> 
> @@ -675,7 +684,8 @@ static int vidioc_querybuf(struct file *file,
>  	struct usbvision_frame *frame;
> 
>  	/* FIXME : must control
> -	   that buffers are mapped (VIDIOC_REQBUFS has been called) */
> +	 * that buffers are mapped (VIDIOC_REQBUFS has been called)
> +	 */
>  	if (vb->index >= usbvision->num_frames)
>  		return -EINVAL;
>  	/* Updating the corresponding frame state */
> @@ -813,6 +823,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
>  					struct v4l2_format *vf)
>  {
>  	struct usb_usbvision *usbvision = video_drvdata(file);
> +
>  	vf->fmt.pix.width = usbvision->curwidth;
>  	vf->fmt.pix.height = usbvision->curheight;
>  	vf->fmt.pix.pixelformat = usbvision->palette.format;
> @@ -897,24 +908,27 @@ static ssize_t usbvision_read(struct file *file, char __user *buf,
>  		return -EFAULT;
> 
>  	/* This entry point is compatible with the mmap routines
> -	   so that a user can do either VIDIOC_QBUF/VIDIOC_DQBUF
> -	   to get frames or call read on the device. */
> +	 * so that a user can do either VIDIOC_QBUF/VIDIOC_DQBUF
> +	 * to get frames or call read on the device.
> +	 */
>  	if (!usbvision->num_frames) {
>  		/* First, allocate some frames to work with
> -		   if this has not been done with VIDIOC_REQBUF */
> +		 * if this has not been done with VIDIOC_REQBUF
> +		 */
>  		usbvision_frames_free(usbvision);
>  		usbvision_empty_framequeues(usbvision);
>  		usbvision_frames_alloc(usbvision, USBVISION_NUMFRAMES);
>  	}
> 
>  	if (usbvision->streaming != stream_on) {
> -		/* no stream is running, make it running ! */
> +		// no stream is running, make it running !
>  		usbvision->streaming = stream_on;
>  		call_all(usbvision, video, s_stream, 1);
>  	}
> 
>  	/* Then, enqueue as many frames as possible
> -	   (like a user of VIDIOC_QBUF would do) */
> +	 * (like a user of VIDIOC_QBUF would do)
> +	 */
>  	for (i = 0; i < usbvision->num_frames; i++) {
>  		frame = &usbvision->frame[i];
>  		if (frame->grabstate == frame_state_unused) {
> @@ -1125,7 +1139,7 @@ static int usbvision_radio_close(struct file *file)
>  	mutex_unlock(&usbvision->v4l2_lock);
> 
>  	if (r) {
> -		printk(KERN_INFO "%s: Final disconnect\n", __func__);
> +		pr_debug("%s: Final disconnect\n", __func__);
>  		v4l2_fh_release(file);
>  		usbvision_release(usbvision);
>  		return 0;
> @@ -1273,7 +1287,7 @@ static int usbvision_register_video(struct usb_usbvision *usbvision)
> 
>  	if (video_register_device(&usbvision->vdev, VFL_TYPE_VIDEO, video_nr) < 0)
>  		goto err_exit;
> -	printk(KERN_INFO "USBVision[%d]: registered USBVision Video device %s [v4l2]\n",
> +	pr_debug("USBVision[%d]: registered USBVision Video device %s [v4l2]\n",
>  	       usbvision->nr, video_device_node_name(&usbvision->vdev));
> 
>  	/* Radio Device: */
> @@ -1284,7 +1298,7 @@ static int usbvision_register_video(struct usb_usbvision *usbvision)
>  		usbvision->rdev.device_caps = V4L2_CAP_RADIO | V4L2_CAP_TUNER;
>  		if (video_register_device(&usbvision->rdev, VFL_TYPE_RADIO, radio_nr) < 0)
>  			goto err_exit;
> -		printk(KERN_INFO "USBVision[%d]: registered USBVision Radio device %s [v4l2]\n",
> +		pr_debug("USBVision[%d]: registered USBVision Radio device %s [v4l2]\n",
>  		       usbvision->nr, video_device_node_name(&usbvision->rdev));
>  	}
>  	/* all done */
> @@ -1429,7 +1443,7 @@ static int usbvision_probe(struct usb_interface *intf,
>  		ret = -ENODEV;
>  		goto err_usb;
>  	}
> -	printk(KERN_INFO "%s: %s found\n", __func__,
> +	pr_debug("%s: %s found\n", __func__,
>  				usbvision_device_data[model].model_string);
> 
>  	if (usbvision_device_data[model].interface >= 0)
> @@ -1501,8 +1515,7 @@ static int usbvision_probe(struct usb_interface *intf,
>  			goto err_pkt;
>  		}
> 
> -		tmp = le16_to_cpu(uif->altsetting[i].endpoint[1].desc.
> -				      wMaxPacketSize);
> +		tmp = le16_to_cpu(uif->altsetting[i].endpoint[1].desc.wMaxPacketSize);
>  		usbvision->alt_max_pkt_size[i] =
>  			(tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);
>  		PDEBUG(DBG_PROBE, "Alternate setting %i, max size= %i", i,
> @@ -1581,7 +1594,7 @@ static void usbvision_disconnect(struct usb_interface *intf)
>  	mutex_unlock(&usbvision->v4l2_lock);
> 
>  	if (u) {
> -		printk(KERN_INFO "%s: In use, disconnect pending\n",
> +		pr_debug("%s: In use, disconnect pending\n",
>  		       __func__);
>  		wake_up_interruptible(&usbvision->wait_frame);
>  		wake_up_interruptible(&usbvision->wait_stream);
> @@ -1625,7 +1638,7 @@ static int __init usbvision_init(void)
>  	err_code = usb_register(&usbvision_driver);
> 
>  	if (err_code == 0) {
> -		printk(KERN_INFO DRIVER_DESC " : " USBVISION_VERSION_STRING "\n");
> +		pr_debug(DRIVER_DESC " : " USBVISION_VERSION_STRING "\n");
>  		PDEBUG(DBG_PROBE, "success");
>  	}
>  	return err_code;
> --
> 2.17.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ