diff mbox series

[2/5] staging: media: av7110: Fix various whitespace checkpatch errors

Message ID 20231020232332.55024-3-bergh.jonathan@gmail.com
State New
Headers
Series staging: media: av7110: Fix various formatting and small code issues |

Commit Message

Jonathan Bergh Oct. 20, 2023, 11:23 p.m. UTC 
  Fixed the following checkpatch errors relating to whitespaces:
 * extra whitespace after '~' symbol
 * remove whitespace before/after parentheses for switch statement and
   function parameters as well as after switch keyword
 * ensure whitespace before '{' braces
 * remove whitespace between function name and '(' parentheses
 * ensure whitespace after ',' commas in parameter lists

Signed-off-by: Jonathan Bergh <bergh.jonathan@gmail.com>
---
 drivers/staging/media/av7110/av7110_av.c | 28 ++++++++++++------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Comments

Dan Carpenter Oct. 23, 2023, 2:26 p.m. UTC | #1 
On Sat, Oct 21, 2023 at 01:23:29AM +0200, Jonathan Bergh wrote:
> Fixed the following checkpatch errors relating to whitespaces:
>  * extra whitespace after '~' symbol
>  * remove whitespace before/after parentheses for switch statement and
>    function parameters as well as after switch keyword
>  * ensure whitespace before '{' braces
>  * remove whitespace between function name and '(' parentheses
>  * ensure whitespace after ',' commas in parameter lists
> 
> Signed-off-by: Jonathan Bergh <bergh.jonathan@gmail.com>
> ---
>  drivers/staging/media/av7110/av7110_av.c | 28 ++++++++++++------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
> index af3845406626..482dfc548b16 100644
> --- a/drivers/staging/media/av7110/av7110_av.c
> +++ b/drivers/staging/media/av7110/av7110_av.c
> @@ -241,8 +241,8 @@ int av7110_pes_play(void *dest, struct dvb_ringbuffer *buf, int dlen)
>  		sync |= DVB_RINGBUFFER_PEEK(buf, 2) << 8;
>  		sync |= DVB_RINGBUFFER_PEEK(buf, 3);
>  
> -		if (((sync &~ 0x0f) == 0x000001e0) ||
> -		    ((sync &~ 0x1f) == 0x000001c0) ||
> +		if (((sync &~0x0f) == 0x000001e0) ||
> +		    ((sync &~0x1f) == 0x000001c0) ||

These should be:

	if (((sync & ~0x0f) == 0x000001e0) ||
	    ((sync & ~0x1f) == 0x000001c0) ||


>  		    (sync == 0x000001bd))
>  			break;
>  		printk("resync\n");

regards,
dan carpenter
Joe Perches Oct. 23, 2023, 3:24 p.m. UTC | #2 
On Mon, 2023-10-23 at 17:26 +0300, Dan Carpenter wrote:
> On Sat, Oct 21, 2023 at 01:23:29AM +0200, Jonathan Bergh wrote:
[]
> > diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
[]
> > @@ -241,8 +241,8 @@ int av7110_pes_play(void *dest, struct dvb_ringbuffer *buf, int dlen)
> >  		sync |= DVB_RINGBUFFER_PEEK(buf, 2) << 8;
> >  		sync |= DVB_RINGBUFFER_PEEK(buf, 3);
> >  
> > -		if (((sync &~ 0x0f) == 0x000001e0) ||
> > -		    ((sync &~ 0x1f) == 0x000001c0) ||
> > +		if (((sync &~0x0f) == 0x000001e0) ||
> > +		    ((sync &~0x1f) == 0x000001c0) ||
> 
> These should be:
> 
> 	if (((sync & ~0x0f) == 0x000001e0) ||
> 	    ((sync & ~0x1f) == 0x000001c0) ||

Generally, I believe it would be better to avoid '& ~constant'
and use the positive form instead

	if ((sync & 0xfffffff0) == 0x000001e0) ||
	    (sync & 0xffffffe0) == 0x000001c0)
Dan Carpenter Oct. 24, 2023, 4:12 a.m. UTC | #3 
On Mon, Oct 23, 2023 at 08:24:58AM -0700, Joe Perches wrote:
> On Mon, 2023-10-23 at 17:26 +0300, Dan Carpenter wrote:
> > On Sat, Oct 21, 2023 at 01:23:29AM +0200, Jonathan Bergh wrote:
> []
> > > diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
> []
> > > @@ -241,8 +241,8 @@ int av7110_pes_play(void *dest, struct dvb_ringbuffer *buf, int dlen)
> > >  		sync |= DVB_RINGBUFFER_PEEK(buf, 2) << 8;
> > >  		sync |= DVB_RINGBUFFER_PEEK(buf, 3);
> > >  
> > > -		if (((sync &~ 0x0f) == 0x000001e0) ||
> > > -		    ((sync &~ 0x1f) == 0x000001c0) ||
> > > +		if (((sync &~0x0f) == 0x000001e0) ||
> > > +		    ((sync &~0x1f) == 0x000001c0) ||
> > 
> > These should be:
> > 
> > 	if (((sync & ~0x0f) == 0x000001e0) ||
> > 	    ((sync & ~0x1f) == 0x000001c0) ||
> 
> Generally, I believe it would be better to avoid '& ~constant'
> and use the positive form instead
> 
> 	if ((sync & 0xfffffff0) == 0x000001e0) ||
> 	    (sync & 0xffffffe0) == 0x000001c0)
> 

To me it depends on if the 0x0f means something.  It would be better as
a define instead of a magic number.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
index af3845406626..482dfc548b16 100644
--- a/drivers/staging/media/av7110/av7110_av.c
+++ b/drivers/staging/media/av7110/av7110_av.c
@@ -241,8 +241,8 @@  int av7110_pes_play(void *dest, struct dvb_ringbuffer *buf, int dlen)
 		sync |= DVB_RINGBUFFER_PEEK(buf, 2) << 8;
 		sync |= DVB_RINGBUFFER_PEEK(buf, 3);
 
-		if (((sync &~ 0x0f) == 0x000001e0) ||
-		    ((sync &~ 0x1f) == 0x000001c0) ||
+		if (((sync &~0x0f) == 0x000001e0) ||
+		    ((sync &~0x1f) == 0x000001c0) ||
 		    (sync == 0x000001bd))
 			break;
 		printk("resync\n");
@@ -599,7 +599,7 @@  static int find_pes_header(u8 const *buf, long int length, int *frags)
 	while (c < length - 3 && !found) {
 		if (buf[c] == 0x00 && buf[c + 1] == 0x00 &&
 		    buf[c + 2] == 0x01) {
-			switch ( buf[c + 3] ) {
+			switch (buf[c + 3]) {
 			case PROG_STREAM_MAP:
 			case PRIVATE_STREAM2:
 			case PROG_STREAM_DIR:
@@ -644,9 +644,9 @@  void av7110_p2t_write(u8 const *buf, long int length, u16 pid, struct av7110_p2t
 
 	c = 0;
 	c2 = 0;
-	if (p->frags){
+	if (p->frags) {
 		check = 0;
-		switch(p->frags) {
+		switch (p->frags) {
 		case 1:
 			if (buf[c] == 0x00 && buf[c + 1] == 0x01) {
 				check = 1;
@@ -712,7 +712,7 @@  void av7110_p2t_write(u8 const *buf, long int length, u16 pid, struct av7110_p2t
 		c2 = find_pes_header(buf + c + add, length - c - add, &p->frags);
 		if (c2 >= 0) {
 			c2 += c + add;
-			if (c2 > c){
+			if (c2 > c) {
 				p_to_t(buf + c, c2 - c, pid, &p->counter, p->feed);
 				c = c2;
 				clear_p2t(p);
@@ -798,7 +798,7 @@  static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
 
 	while (c < length) {
 		memset(obuf, 0, TS_SIZE);
-		if (length - c >= (TS_SIZE - 4)){
+		if (length - c >= (TS_SIZE - 4)) {
 			l = write_ts_header2(pid, counter, pes_start,
 					     obuf, (TS_SIZE - 4));
 			memcpy(obuf + l, buf + c, TS_SIZE - l);
@@ -896,7 +896,7 @@  void dvb_video_add_event(struct av7110 *av7110, struct video_event *event)
 }
 
 
-static int dvb_video_get_event (struct av7110 *av7110, struct video_event *event, int flags)
+static int dvb_video_get_event(struct av7110 *av7110, struct video_event *event, int flags)
 {
 	struct dvb_video_events *events = &av7110->video_events;
 
@@ -1135,11 +1135,11 @@  static int dvb_video_ioctl(struct file *file,
 	unsigned long arg = (unsigned long) parg;
 	int ret = 0;
 
-	dprintk(1, "av7110:%p, cmd=%04x\n", av7110,cmd);
+	dprintk(1, "av7110:%p, cmd=%04x\n", av7110, cmd);
 
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
-		if ( cmd != VIDEO_GET_STATUS && cmd != VIDEO_GET_EVENT &&
-		     cmd != VIDEO_GET_SIZE ) {
+		if (cmd != VIDEO_GET_STATUS && cmd != VIDEO_GET_EVENT &&
+		     cmd != VIDEO_GET_SIZE) {
 			return -EPERM;
 		}
 	}
@@ -1367,7 +1367,7 @@  static int dvb_audio_ioctl(struct file *file,
 	unsigned long arg = (unsigned long) parg;
 	int ret = 0;
 
-	dprintk(1, "av7110:%p, cmd=%04x\n", av7110,cmd);
+	dprintk(1, "av7110:%p, cmd=%04x\n", av7110, cmd);
 
 	if (((file->f_flags & O_ACCMODE) == O_RDONLY) &&
 	    (cmd != AUDIO_GET_STATUS))
@@ -1433,7 +1433,7 @@  static int dvb_audio_ioctl(struct file *file,
 
 	case AUDIO_CHANNEL_SELECT:
 		av7110->audiostate.channel_select = (audio_channel_select_t) arg;
-		switch(av7110->audiostate.channel_select) {
+		switch (av7110->audiostate.channel_select) {
 		case AUDIO_STEREO:
 			ret = audcom(av7110, AUDIO_CMD_STEREO);
 			if (!ret) {
@@ -1642,7 +1642,7 @@  int av7110_av_register(struct av7110 *av7110)
 	spin_lock_init(&av7110->video_events.lock);
 	av7110->video_events.eventw = av7110->video_events.eventr = 0;
 	av7110->video_events.overflow = 0;
-	memset(&av7110->video_size, 0, sizeof (video_size_t));
+	memset(&av7110->video_size, 0, sizeof(video_size_t));
 
 	dvb_register_device(&av7110->dvb_adapter, &av7110->video_dev,
 			    &dvbdev_video, av7110, DVB_DEVICE_VIDEO, 0);