[PATCH] staging: iio: cleanup ring_sw.c

[PATCH] staging: iio: cleanup ring_sw.c

[Cong Ding]

clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line
issue.

Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 drivers/staging/iio/ring_sw.c |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/ring_sw.c b/drivers/staging/iio/ring_sw.c
index 3a45f9a..4db1cc3 100644
--- a/drivers/staging/iio/ring_sw.c
+++ b/drivers/staging/iio/ring_sw.c
@@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 		 * as long as the read pointer is valid before this
 		 * passes it - guaranteed as set later in this function.
 		 */
-		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
+		ring->half_p = ring->data -
+			ring->buf.length*ring->buf.bytes_per_datum/2;
 	}
 	/* Copy data to where ever the current write pointer says */
 	memcpy(ring->write_p, data, ring->buf.bytes_per_datum);
@@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	else if (ring->write_p == ring->read_p) {
 		change_test_ptr = ring->read_p;
 		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
-		if (temp_ptr
-		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
+		if (temp_ptr == ring->data +
+		    ring->buf.length*ring->buf.bytes_per_datum) {
 			temp_ptr = ring->data;
 		}
 		/* We are moving pointer on one because the ring is full.  Any
@@ -127,7 +128,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
 	 * simultaneous read */
 	/* Also need to use loop count to ensure this only happens once */
 	ring->half_p += ring->buf.bytes_per_datum;
-	if (ring->half_p == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
+	if (ring->half_p == ring->data +
+			ring->buf.length*ring->buf.bytes_per_datum)
 		ring->half_p = ring->data;
 	if (ring->half_p == ring->read_p) {
 		ring->buf.stufftoread = true;
@@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (n % ring->buf.bytes_per_datum) {
 		ret = -EINVAL;
 		printk(KERN_INFO "Ring buffer read request not whole number of"
-		       "samples: Request bytes %zd, Current bytes per datum %d\n",
+		       "samples: Request bytes %zd, Current bytes per"
+		       "datum %d\n",
 		       n, ring->buf.bytes_per_datum);
 		goto error_ret;
 	}
@@ -193,7 +196,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_write_p >= initial_read_p)
 		data_available = initial_write_p - initial_read_p;
 	else
-		data_available = buffer_size - (initial_read_p - initial_write_p);
+		data_available = buffer_size -
+			(initial_read_p - initial_write_p);
 
 	if (data_available < bytes_to_rip)
 		bytes_to_rip = data_available;
@@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
 		max_copied = ring->data + buffer_size - initial_read_p;
 		memcpy(data, initial_read_p, max_copied);
-		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
+		memcpy(data + max_copied,
+				ring->data, bytes_to_rip - max_copied);
 		end_read_p = ring->data + bytes_to_rip - max_copied;
 	} else {
 		memcpy(data, initial_read_p, bytes_to_rip);
@@ -251,12 +256,10 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
 error_free_data_cpy:
 	kfree(data);
 error_ret:
-
 	return ret;
 }
 
-static int iio_store_to_sw_rb(struct iio_buffer *r,
-			      u8 *data)
+static int iio_store_to_sw_rb(struct iio_buffer *r, u8 *data)
 {
 	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
 	return iio_store_to_sw_ring(ring, data);
@@ -343,7 +346,7 @@ struct iio_buffer *iio_sw_rb_allocate(struct iio_dev *indio_dev)
 	struct iio_buffer *buf;
 	struct iio_sw_ring_buffer *ring;
 
-	ring = kzalloc(sizeof *ring, GFP_KERNEL);
+	ring = kzalloc(sizeof(*ring), GFP_KERNEL);
 	if (!ring)
 		return NULL;
 	ring->update_needed = true;
-- 
1.7.10.4

Re: [PATCH] staging: iio: cleanup ring_sw.c

[Dan Carpenter]

On Wed, Dec 19, 2012 at 12:39:59AM +0100, Cong Ding wrote:
> clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line
> issue.
> 
> Signed-off-by: Cong Ding <dinggnu@gmail.com>
> ---
>  drivers/staging/iio/ring_sw.c |   25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/ring_sw.c b/drivers/staging/iio/ring_sw.c
> index 3a45f9a..4db1cc3 100644
> --- a/drivers/staging/iio/ring_sw.c
> +++ b/drivers/staging/iio/ring_sw.c
> @@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
>  		 * as long as the read pointer is valid before this
>  		 * passes it - guaranteed as set later in this function.
>  		 */
> -		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
> +		ring->half_p = ring->data -
> +			ring->buf.length*ring->buf.bytes_per_datum/2;

Please put spaces around the '*' and '/' characters.
			ring->buf.length * ring->buf.bytes_per_datum / 2;

>  	}
>  	/* Copy data to where ever the current write pointer says */
>  	memcpy(ring->write_p, data, ring->buf.bytes_per_datum);
> @@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
>  	else if (ring->write_p == ring->read_p) {
>  		change_test_ptr = ring->read_p;
>  		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
> -		if (temp_ptr
> -		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
> +		if (temp_ptr == ring->data +
> +		    ring->buf.length*ring->buf.bytes_per_datum) {

This needs spaces as well.  It might look cleaner if you broke it
up like this:
		if (temp_ptr == ring->data + ring->buf.length *
				ring->buf.bytes_per_datum) {


>  			temp_ptr = ring->data;
>  		}
>  		/* We are moving pointer on one because the ring is full.  Any
> @@ -127,7 +128,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
>  	 * simultaneous read */
>  	/* Also need to use loop count to ensure this only happens once */
>  	ring->half_p += ring->buf.bytes_per_datum;
> -	if (ring->half_p == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
> +	if (ring->half_p == ring->data +
> +			ring->buf.length*ring->buf.bytes_per_datum)

Same.

>  		ring->half_p = ring->data;
>  	if (ring->half_p == ring->read_p) {
>  		ring->buf.stufftoread = true;
> @@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
>  	if (n % ring->buf.bytes_per_datum) {
>  		ret = -EINVAL;
>  		printk(KERN_INFO "Ring buffer read request not whole number of"
> -		       "samples: Request bytes %zd, Current bytes per datum %d\n",
> +		       "samples: Request bytes %zd, Current bytes per"
> +		       "datum %d\n",

No space between "of" and "samples" and also "per" and "datum".

The warning here is that the print should be on one line instead of
two.  But actually that is a decision for the maintainer.
Checkpatch.pl is not the king of us, we do not have to do what it
says.

>  		       n, ring->buf.bytes_per_datum);
>  		goto error_ret;
>  	}
> @@ -193,7 +196,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
>  	if (initial_write_p >= initial_read_p)
>  		data_available = initial_write_p - initial_read_p;
>  	else
> -		data_available = buffer_size - (initial_read_p - initial_write_p);
> +		data_available = buffer_size -
> +			(initial_read_p - initial_write_p);
>  
>  	if (data_available < bytes_to_rip)
>  		bytes_to_rip = data_available;
> @@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
>  	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
>  		max_copied = ring->data + buffer_size - initial_read_p;
>  		memcpy(data, initial_read_p, max_copied);
> -		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
> +		memcpy(data + max_copied,
> +				ring->data, bytes_to_rip - max_copied);

Line it up like this:
		memcpy(data + max_copied, ring->data,
		       bytes_to_rip - max_copied);


[tab][tab][space][space][space][space][space][space][space]bytes_

regards,
dan carpenter

Re: [PATCH] staging: iio: cleanup ring_sw.c

[Joe Perches]

On Wed, 2012-12-19 at 09:42 +0300, Dan Carpenter wrote:
> On Wed, Dec 19, 2012 at 12:39:59AM +0100, Cong Ding wrote:
> > clean the checkpatch warnings in ring_sw.c. mostly are 80 characters per line
> > issue.
[]
> > diff --git a/drivers/staging/iio/ring_sw.c b/drivers/staging/iio/ring_sw.c
[]
> > @@ -77,7 +77,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
> >  		 * as long as the read pointer is valid before this
> >  		 * passes it - guaranteed as set later in this function.
> >  		 */
> > -		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
> > +		ring->half_p = ring->data -
> > +			ring->buf.length*ring->buf.bytes_per_datum/2;
> 
> Please put spaces around the '*' and '/' characters.
> 			ring->buf.length * ring->buf.bytes_per_datum / 2;

or maybe add a helper like ring_datum_size(ring):

size_t ring_datum_size(const struct iio_sw_ring_buffer *ring)
{
	return ring->buf.length * ring->buf.bytes_per_datum;
}

so this becomes

	ring->half_p = ring->data - ring_datum_size(ring) / 2;

> > @@ -112,8 +113,8 @@ static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
> >  	else if (ring->write_p == ring->read_p) {
> >  		change_test_ptr = ring->read_p;
> >  		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
> > -		if (temp_ptr
> > -		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
> > +		if (temp_ptr == ring->data +
> > +		    ring->buf.length*ring->buf.bytes_per_datum) {

> 
> This needs spaces as well.  It might look cleaner if you broke it
> up like this:
> 		if (temp_ptr == ring->data + ring->buf.length *
> 				ring->buf.bytes_per_datum) {

or
		if (temp_ptr == ring->data + ring_datum_size(ring)) {

etc...

> > @@ -153,7 +155,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
> >  	if (n % ring->buf.bytes_per_datum) {
> >  		ret = -EINVAL;
> >  		printk(KERN_INFO "Ring buffer read request not whole number of"
> > -		       "samples: Request bytes %zd, Current bytes per datum %d\n",
> > +		       "samples: Request bytes %zd, Current bytes per"
> > +		       "datum %d\n",
> 
> No space between "of" and "samples" and also "per" and "datum".
> 
> The warning here is that the print should be on one line instead of
> two.  But actually that is a decision for the maintainer.
> Checkpatch.pl is not the king of us, we do not have to do what it
> says.

Yup.

It is nicer though to coalesce formats so that it's easier to
grep for strings and substrings.

> > @@ -201,7 +205,8 @@ static int iio_read_first_n_sw_rb(struct iio_buffer *r,
> >  	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
> >  		max_copied = ring->data + buffer_size - initial_read_p;
> >  		memcpy(data, initial_read_p, max_copied);
> > -		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
> > +		memcpy(data + max_copied,
> > +				ring->data, bytes_to_rip - max_copied);
> 
> Line it up like this:
> 		memcpy(data + max_copied, ring->data,
> 		       bytes_to_rip - max_copied);
> 
> 
> [tab][tab][space][space][space][space][space][space][space]bytes_

checkpatch --strict emits alignment messages

or not worry about 80 columns in this case.

Re: [PATCH] staging: iio: cleanup ring_sw.c

[Joe Perches]

On Wed, 2012-12-19 at 07:13 +0000, Jonathan Cameron wrote:
> Err. This whole file is going away in the coming cycle anyway so I am
> not going to take cleanup patches for it.

That's the best kind of staging cleanup.