[2/4] staging: r8188eu: reformat long computation lines

  Reformat long running computation instructions to improve code readability.
Address following checkpatch script complaints:
	CHECK: line length of 171 exceeds 100 columns
	CHECK: line length of 113 exceeds 100 columns

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

--
2.30.2
  

On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> Reformat long running computation instructions to improve code readability.
> Address following checkpatch script complaints:
> 	CHECK: line length of 171 exceeds 100 columns
> 	CHECK: line length of 113 exceeds 100 columns
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index 79daf8f269d6..427da7e8ba4c 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
>  	} else if (network_addr[0] == NAT25_IPX) {
>  		unsigned long x;
> 
> -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
> -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^ network_addr[10];
> +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^

Why not go out to [4] here and then you are one line shorter?

> +		    network_addr[4] ^ network_addr[5] ^ network_addr[6] ^
> +		    network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> +		    network_addr[10];
> 
>  		return x & (NAT25_HASH_SIZE - 1);
>  	} else if (network_addr[0] == NAT25_APPLE) {
> @@ -224,16 +226,20 @@ static int __nat25_network_hash(unsigned char *network_addr)
>  	} else if (network_addr[0] == NAT25_PPPOE) {
>  		unsigned long x;
> 
> -		x = network_addr[0] ^ network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^ network_addr[6] ^ network_addr[7] ^ network_addr[8];
> +		x = network_addr[0] ^ network_addr[1] ^ network_addr[2] ^
> +		    network_addr[3] ^ network_addr[4] ^ network_addr[5] ^

Same here


> +		    network_addr[6] ^ network_addr[7] ^ network_addr[8];
> 
>  		return x & (NAT25_HASH_SIZE - 1);
>  	} else if (network_addr[0] == NAT25_IPV6) {
>  		unsigned long x;
> 
> -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
> -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^ network_addr[10] ^
> -			network_addr[11] ^ network_addr[12] ^ network_addr[13] ^ network_addr[14] ^ network_addr[15] ^
> -			network_addr[16];
> +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> +		    network_addr[4] ^ network_addr[5] ^ network_addr[6] ^
> +		    network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> +		    network_addr[10] ^ network_addr[11] ^ network_addr[12] ^
> +		    network_addr[13] ^ network_addr[14] ^ network_addr[15] ^
> +		    network_addr[16];

And here.

thanks,

greg k-h
  

On Mon, Oct 17, 2022 at 04:09:49PM +0200, Greg KH wrote:
> On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > Reformat long running computation instructions to improve code readability.
> > Address following checkpatch script complaints:
> > 	CHECK: line length of 171 exceeds 100 columns
> > 	CHECK: line length of 113 exceeds 100 columns
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 79daf8f269d6..427da7e8ba4c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> >  	} else if (network_addr[0] == NAT25_IPX) {
> >  		unsigned long x;
> >
> > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
> > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^ network_addr[10];
> > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
>
> Why not go out to [4] here and then you are one line shorter?

Thank you for the feedback.
Arranging 4 on a line still made the line extend 90+ columns. It appeared wide
to me. Stacking three in one line made it look better.
I will resend the patch with 4 in line and let you suggest if that is
acceptable.

Thank you,
./drv

>
> > +		    network_addr[4] ^ network_addr[5] ^ network_addr[6] ^
> > +		    network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > +		    network_addr[10];
> >
> >  		return x & (NAT25_HASH_SIZE - 1);
> >  	} else if (network_addr[0] == NAT25_APPLE) {
> > @@ -224,16 +226,20 @@ static int __nat25_network_hash(unsigned char *network_addr)
> >  	} else if (network_addr[0] == NAT25_PPPOE) {
> >  		unsigned long x;
> >
> > -		x = network_addr[0] ^ network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^ network_addr[6] ^ network_addr[7] ^ network_addr[8];
> > +		x = network_addr[0] ^ network_addr[1] ^ network_addr[2] ^
> > +		    network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
>
> Same here
>
>
> > +		    network_addr[6] ^ network_addr[7] ^ network_addr[8];
> >
> >  		return x & (NAT25_HASH_SIZE - 1);
> >  	} else if (network_addr[0] == NAT25_IPV6) {
> >  		unsigned long x;
> >
> > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
> > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^ network_addr[10] ^
> > -			network_addr[11] ^ network_addr[12] ^ network_addr[13] ^ network_addr[14] ^ network_addr[15] ^
> > -			network_addr[16];
> > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > +		    network_addr[4] ^ network_addr[5] ^ network_addr[6] ^
> > +		    network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > +		    network_addr[10] ^ network_addr[11] ^ network_addr[12] ^
> > +		    network_addr[13] ^ network_addr[14] ^ network_addr[15] ^
> > +		    network_addr[16];
>
> And here.
>
> thanks,
>
> greg k-h
>
  

On Mon, Oct 17, 2022 at 07:40:46PM +0530, Deepak R Varma wrote:
> On Mon, Oct 17, 2022 at 04:09:49PM +0200, Greg KH wrote:
> > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > Reformat long running computation instructions to improve code readability.
> > > Address following checkpatch script complaints:
> > > 	CHECK: line length of 171 exceeds 100 columns
> > > 	CHECK: line length of 113 exceeds 100 columns
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > index 79daf8f269d6..427da7e8ba4c 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > >  	} else if (network_addr[0] == NAT25_IPX) {
> > >  		unsigned long x;
> > >
> > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^ network_addr[5] ^
> > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^ network_addr[10];
> > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> >
> > Why not go out to [4] here and then you are one line shorter?
> 
> Thank you for the feedback.
> Arranging 4 on a line still made the line extend 90+ columns.

As the tool said, you can go up to 100 columns.

thanks,

greg k-h
  

From: Greg KH
> Sent: 17 October 2022 15:10
> 
> On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > Reformat long running computation instructions to improve code readability.
> > Address following checkpatch script complaints:
> > 	CHECK: line length of 171 exceeds 100 columns
> > 	CHECK: line length of 113 exceeds 100 columns
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 79daf8f269d6..427da7e8ba4c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> >  	} else if (network_addr[0] == NAT25_IPX) {
> >  		unsigned long x;
> >
> > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> network_addr[5] ^
> > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> network_addr[10];
> > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> 
> Why not go out to [4] here and then you are one line shorter?

and/or use a shorter variable name....

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
  

On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> From: Greg KH
> > Sent: 17 October 2022 15:10
> >
> > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > Reformat long running computation instructions to improve code readability.
> > > Address following checkpatch script complaints:
> > > 	CHECK: line length of 171 exceeds 100 columns
> > > 	CHECK: line length of 113 exceeds 100 columns
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > index 79daf8f269d6..427da7e8ba4c 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > >  	} else if (network_addr[0] == NAT25_IPX) {
> > >  		unsigned long x;
> > >
> > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > network_addr[5] ^
> > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > network_addr[10];
> > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> >
> > Why not go out to [4] here and then you are one line shorter?
>
> and/or use a shorter variable name....
Hi David,
I have already re-submitted the patch set with 4 in line arrangement. Do you
still suggest using shorter variable names?

Thank you,
./drv

>
> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>
  

On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > From: Greg KH
> > > Sent: 17 October 2022 15:10
> > > 
> > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > Reformat long running computation instructions to improve code readability.
> > > > Address following checkpatch script complaints:
> > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > 	CHECK: line length of 113 exceeds 100 columns
[]
> > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
[]
> > > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > > >  	} else if (network_addr[0] == NAT25_IPX) {
> > > >  		unsigned long x;
> > > > 
> > > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > > network_addr[5] ^
> > > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > > network_addr[10];
> > > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > > 
> > > Why not go out to [4] here and then you are one line shorter?
> > 
> > and/or use a shorter variable name....
> Hi David,
> I have already re-submitted the patch set with 4 in line arrangement. Do you
> still suggest using shorter variable names?

Assuming this code is not performance sensitive, I suggest not just
molifying checkpatch but perhaps improving the code by adding a helper
function something like:

u8 xor_array_u8(u8 *x, size_t len)
{
	size_t i;
	u8 xor = x[0];

	for (i = 1; i < len; i++)
		xor ^= x[i];

	return xor;
}

so for instance this could be:

		x = xor_array_u8(&network_addr[1], 10);
  

On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > From: Greg KH
> > > > Sent: 17 October 2022 15:10
> > > >
> > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > Reformat long running computation instructions to improve code readability.
> > > > > Address following checkpatch script complaints:
> > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > 	CHECK: line length of 113 exceeds 100 columns
> []
> > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> []
> > > > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > > > >  	} else if (network_addr[0] == NAT25_IPX) {
> > > > >  		unsigned long x;
> > > > >
> > > > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > > > network_addr[5] ^
> > > > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > > > network_addr[10];
> > > > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > > >
> > > > Why not go out to [4] here and then you are one line shorter?
> > >
> > > and/or use a shorter variable name....
> > Hi David,
> > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > still suggest using shorter variable names?
>
> Assuming this code is not performance sensitive, I suggest not just
> molifying checkpatch but perhaps improving the code by adding a helper
> function something like:
>
> u8 xor_array_u8(u8 *x, size_t len)
> {
> 	size_t i;
> 	u8 xor = x[0];
>
> 	for (i = 1; i < len; i++)
> 		xor ^= x[i];
>
> 	return xor;
> }
>
> so for instance this could be:
>
> 		x = xor_array_u8(&network_addr[1], 10);
>

Hi Joe,
Great suggestion. Thank you.
Is there a way to confirm that this improvement won't impact performance? Will I
need any specific hardware / device to run tests?

./drv
  

On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > From: Greg KH
> > > > > Sent: 17 October 2022 15:10
> > > > > 
> > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > Address following checkpatch script complaints:
> > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > []
> > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > []
> > > > > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > > > > >  	} else if (network_addr[0] == NAT25_IPX) {
> > > > > >  		unsigned long x;
> > > > > > 
> > > > > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > > > > network_addr[5] ^
> > > > > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > > > > network_addr[10];
> > > > > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > > > > 
> > > > > Why not go out to [4] here and then you are one line shorter?
> > > > 
> > > > and/or use a shorter variable name....
> > > Hi David,
> > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > still suggest using shorter variable names?
> > 
> > Assuming this code is not performance sensitive, I suggest not just
> > molifying checkpatch but perhaps improving the code by adding a helper
> > function something like:
> > 
> > u8 xor_array_u8(u8 *x, size_t len)
> > {
> > 	size_t i;
> > 	u8 xor = x[0];
> > 
> > 	for (i = 1; i < len; i++)
> > 		xor ^= x[i];
> > 
> > 	return xor;
> > }
> > 
> > so for instance this could be:
> > 
> > 		x = xor_array_u8(&network_addr[1], 10);
> > 
> 
> Hi Joe,
> Great suggestion. Thank you.
> Is there a way to confirm that this improvement won't impact performance? Will I
> need any specific hardware / device to run tests?

I suggest reading the code to see if the uses are in some fast path.
  

On Tue, Oct 18, 2022 at 11:38:22PM -0700, Joe Perches wrote:
> On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> > On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > > From: Greg KH
> > > > > > Sent: 17 October 2022 15:10
> > > > > >
> > > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > > Address following checkpatch script complaints:
> > > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > > []
> > > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > []
> > > > > > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > > > > > >  	} else if (network_addr[0] == NAT25_IPX) {
> > > > > > >  		unsigned long x;
> > > > > > >
> > > > > > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > > > > > network_addr[5] ^
> > > > > > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > > > > > network_addr[10];
> > > > > > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > > > > >
> > > > > > Why not go out to [4] here and then you are one line shorter?
> > > > >
> > > > > and/or use a shorter variable name....
> > > > Hi David,
> > > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > > still suggest using shorter variable names?
> > >
> > > Assuming this code is not performance sensitive, I suggest not just
> > > molifying checkpatch but perhaps improving the code by adding a helper
> > > function something like:
> > >
> > > u8 xor_array_u8(u8 *x, size_t len)
> > > {
> > > 	size_t i;
> > > 	u8 xor = x[0];
> > >
> > > 	for (i = 1; i < len; i++)
> > > 		xor ^= x[i];
> > >
> > > 	return xor;
> > > }
> > >
> > > so for instance this could be:
> > >
> > > 		x = xor_array_u8(&network_addr[1], 10);
> > >
> >
> > Hi Joe,
> > Great suggestion. Thank you.
> > Is there a way to confirm that this improvement won't impact performance? Will I
> > need any specific hardware / device to run tests?
>
> I suggest reading the code to see if the uses are in some fast path.

Sounds good. Thank you for your guidance.

>
  

On Wed, Oct 19, 2022 at 12:14:38PM +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 11:38:22PM -0700, Joe Perches wrote:
> > On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> > > On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > > > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > > > From: Greg KH
> > > > > > > Sent: 17 October 2022 15:10
> > > > > > >
> > > > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > > > Address following checkpatch script complaints:
> > > > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > > > []
> > > > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > > []
> > > > > > > > @@ -211,8 +211,10 @@ static int __nat25_network_hash(unsigned char *network_addr)
> > > > > > > >  	} else if (network_addr[0] == NAT25_IPX) {
> > > > > > > >  		unsigned long x;
> > > > > > > >
> > > > > > > > -		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^ network_addr[4] ^
> > > > > > > network_addr[5] ^
> > > > > > > > -			network_addr[6] ^ network_addr[7] ^ network_addr[8] ^ network_addr[9] ^
> > > > > > > network_addr[10];
> > > > > > > > +		x = network_addr[1] ^ network_addr[2] ^ network_addr[3] ^
> > > > > > >
> > > > > > > Why not go out to [4] here and then you are one line shorter?
> > > > > >
> > > > > > and/or use a shorter variable name....
> > > > > Hi David,
> > > > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > > > still suggest using shorter variable names?
> > > >
> > > > Assuming this code is not performance sensitive, I suggest not just
> > > > molifying checkpatch but perhaps improving the code by adding a helper
> > > > function something like:
> > > >
> > > > u8 xor_array_u8(u8 *x, size_t len)
> > > > {
> > > > 	size_t i;
> > > > 	u8 xor = x[0];
> > > >
> > > > 	for (i = 1; i < len; i++)
> > > > 		xor ^= x[i];
> > > >
> > > > 	return xor;
> > > > }
> > > >
> > > > so for instance this could be:
> > > >
> > > > 		x = xor_array_u8(&network_addr[1], 10);
> > > >
> > >
> > > Hi Joe,
> > > Great suggestion. Thank you.
> > > Is there a way to confirm that this improvement won't impact performance? Will I
> > > need any specific hardware / device to run tests?
> >
> > I suggest reading the code to see if the uses are in some fast path.
>
> Sounds good. Thank you for your guidance.

Hi Joe,
based on the code review so far, I am unable to determine if the chain of
function calls are part of any fast path. There is not enough code comments or
documentation available with this code.

Considering my Outreachy patch submission targets and timelines, I am unable to
spend much time on this research right now; unless an expert can confirm it is
okay to add the routine you outlined. Else, I will put this in on my TODO list
and revisit when I have time.

R8188EU maintainers / experts,
Can you confirm if it is sensible to implement the helper function suggested by
Joe? If yes, I will include the improvement in my current patch set and resubmit
the set for review.

Thank you,
./drv






>
> >
>
>
>