2021-06-16 09:38:46

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 0/8] net: hdlc_ppp: clean up some code style issues

From: Peng Li <[email protected]>

This patchset clean up some code style issues.

Peng Li (8):
net: hdlc_ppp: remove redundant blank lines
net: hdlc_ppp: add blank line after declarations
net: hdlc_ppp: fix the code style issue about "foo* bar"
net: hdlc_ppp: move out assignment in if condition
net: hdlc_ppp: fix the comments style issue
net: hdlc_ppp: remove unnecessary out of memory message
net: hdlc_ppp: remove redundant spaces
net: hdlc_ppp: add required space

drivers/net/wan/hdlc_ppp.c | 90 ++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 48 deletions(-)

--
2.8.1


2021-06-16 09:38:56

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 8/8] net: hdlc_ppp: add required space

From: Peng Li <[email protected]>

Add space required after that ','.

Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/wan/hdlc_ppp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 53c668e..bef7373 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -676,7 +676,8 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)

/* no settable parameters */

- result = hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
+ result = hdlc->attach(dev, ENCODING_NRZ,
+ PARITY_CRC16_PR1_CCITT);
if (result)
return result;

--
2.8.1

2021-06-16 09:39:07

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 4/8] net: hdlc_ppp: move out assignment in if condition

From: Peng Li <[email protected]>

Should not use assignment in if condition.

Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/wan/hdlc_ppp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 8614912..fb5102c 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -375,7 +375,8 @@ static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
u8 *out;
unsigned int len = req_len, nak_len = 0, rej_len = 0;

- if (!(out = kmalloc(len, GFP_ATOMIC))) {
+ out = kmalloc(len, GFP_ATOMIC);
+ if (!out) {
dev->stats.rx_dropped++;
return; /* out of memory, ignore CR packet */
}
--
2.8.1

2021-06-16 09:39:52

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 5/8] net: hdlc_ppp: fix the comments style issue

From: Peng Li <[email protected]>

Networking block comments don't use an empty /* line,
use /* Comment...

Block comments use * on subsequent lines.
Block comments use a trailing */ on a separate line.

This patch fixes the comments style issues.

Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/wan/hdlc_ppp.c | 43 +++++++++++++++++++++----------------------
1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index fb5102c..9bda2c5 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -257,27 +257,25 @@ static void ppp_tx_cp(struct net_device *dev, u16 pid, u8 code,
}

/* State transition table (compare STD-51)
- Events Actions
- TO+ = Timeout with counter > 0 irc = Initialize-Restart-Count
- TO- = Timeout with counter expired zrc = Zero-Restart-Count
-
- RCR+ = Receive-Configure-Request (Good) scr = Send-Configure-Request
- RCR- = Receive-Configure-Request (Bad)
- RCA = Receive-Configure-Ack sca = Send-Configure-Ack
- RCN = Receive-Configure-Nak/Rej scn = Send-Configure-Nak/Rej
-
- RTR = Receive-Terminate-Request str = Send-Terminate-Request
- RTA = Receive-Terminate-Ack sta = Send-Terminate-Ack
-
- RUC = Receive-Unknown-Code scj = Send-Code-Reject
- RXJ+ = Receive-Code-Reject (permitted)
- or Receive-Protocol-Reject
- RXJ- = Receive-Code-Reject (catastrophic)
- or Receive-Protocol-Reject
-*/
+ * Events Actions
+ * TO+ = Timeout with counter > 0 irc = Initialize-Restart-Count
+ * TO- = Timeout with counter expired zrc = Zero-Restart-Count
+ * RCR+ = Receive-Configure-Request (Good) scr = Send-Configure-Request
+ * RCR- = Receive-Configure-Request (Bad)
+ * RCA = Receive-Configure-Ack sca = Send-Configure-Ack
+ * RCN = Receive-Configure-Nak/Rej scn = Send-Configure-Nak/Rej
+ * RTR = Receive-Terminate-Request str = Send-Terminate-Request
+ * RTA = Receive-Terminate-Ack sta = Send-Terminate-Ack
+ * RUC = Receive-Unknown-Code scj = Send-Code-Reject
+ * RXJ+ = Receive-Code-Reject (permitted)
+ * or Receive-Protocol-Reject
+ * RXJ- = Receive-Code-Reject (catastrophic)
+ * or Receive-Protocol-Reject
+ */
static int cp_table[EVENTS][STATES] = {
/* CLOSED STOPPED STOPPING REQ_SENT ACK_RECV ACK_SENT OPENED
- 0 1 2 3 4 5 6 */
+ * 0 1 2 3 4 5 6
+ */
{IRC|SCR|3, INV , INV , INV , INV , INV , INV }, /* START */
{ INV , 0 , 0 , 0 , 0 , 0 , 0 }, /* STOP */
{ INV , INV ,STR|2, SCR|3 ,SCR|3, SCR|5 , INV }, /* TO+ */
@@ -294,9 +292,10 @@ static int cp_table[EVENTS][STATES] = {
};

/* SCA: RCR+ must supply id, len and data
- SCN: RCR- must supply code, id, len and data
- STA: RTR must supply id
- SCJ: RUC must supply CP packet len and data */
+ * SCN: RCR- must supply code, id, len and data
+ * STA: RTR must supply id
+ * SCJ: RUC must supply CP packet len and data
+ */
static void ppp_cp_event(struct net_device *dev, u16 pid, u16 event, u8 code,
u8 id, unsigned int len, const void *data)
{
--
2.8.1

2021-06-16 09:39:53

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 7/8] net: hdlc_ppp: remove redundant spaces

From: Peng Li <[email protected]>

According to the chackpatch.pl,
no spaces is necessary at the start of a line.

Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/wan/hdlc_ppp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 7b7c02d..53c668e 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -34,8 +34,8 @@

enum {IDX_LCP = 0, IDX_IPCP, IDX_IPV6CP, IDX_COUNT};
enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
- CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
- LCP_DISC_REQ, CP_CODES};
+ CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
+ LCP_DISC_REQ, CP_CODES};
#if DEBUG_CP
static const char *const code_names[CP_CODES] = {
"0", "ConfReq", "ConfAck", "ConfNak", "ConfRej", "TermReq",
@@ -80,11 +80,11 @@ struct ppp {
};

enum {CLOSED = 0, STOPPED, STOPPING, REQ_SENT, ACK_RECV, ACK_SENT, OPENED,
- STATES, STATE_MASK = 0xF};
+ STATES, STATE_MASK = 0xF};
enum {START = 0, STOP, TO_GOOD, TO_BAD, RCR_GOOD, RCR_BAD, RCA, RCN, RTR, RTA,
- RUC, RXJ_GOOD, RXJ_BAD, EVENTS};
+ RUC, RXJ_GOOD, RXJ_BAD, EVENTS};
enum {INV = 0x10, IRC = 0x20, ZRC = 0x40, SCR = 0x80, SCA = 0x100,
- SCN = 0x200, STR = 0x400, STA = 0x800, SCJ = 0x1000};
+ SCN = 0x200, STR = 0x400, STA = 0x800, SCJ = 0x1000};

#if DEBUG_STATE
static const char *const state_names[STATES] = {
--
2.8.1

2021-06-16 09:39:58

by Guangbin Huang

[permalink] [raw]
Subject: [PATCH net-next 6/8] net: hdlc_ppp: remove unnecessary out of memory message

From: Peng Li <[email protected]>

This patch removes unnecessary out of memory message,
to fix the following checkpatch.pl warning:
"WARNING: Possible unnecessary 'out of memory' message"

Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/wan/hdlc_ppp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 9bda2c5..7b7c02d 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -219,10 +219,9 @@ static void ppp_tx_cp(struct net_device *dev, u16 pid, u8 code,

skb = dev_alloc_skb(sizeof(struct hdlc_header) +
sizeof(struct cp_header) + magic_len + len);
- if (!skb) {
- netdev_warn(dev, "out of memory in ppp_tx_cp()\n");
+ if (!skb)
return;
- }
+
skb_reserve(skb, sizeof(struct hdlc_header));

cp = skb_put(skb, sizeof(struct cp_header));
--
2.8.1

2021-06-16 22:50:59

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 7/8] net: hdlc_ppp: remove redundant spaces

On Wed, Jun 16, 2021 at 05:33:56PM +0800, Guangbin Huang wrote:
> From: Peng Li <[email protected]>
>
> According to the chackpatch.pl,
> no spaces is necessary at the start of a line.
>
> Signed-off-by: Peng Li <[email protected]>
> Signed-off-by: Guangbin Huang <[email protected]>
> ---
> drivers/net/wan/hdlc_ppp.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
> index 7b7c02d..53c668e 100644
> --- a/drivers/net/wan/hdlc_ppp.c
> +++ b/drivers/net/wan/hdlc_ppp.c
> @@ -34,8 +34,8 @@
>
> enum {IDX_LCP = 0, IDX_IPCP, IDX_IPV6CP, IDX_COUNT};
> enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
> - CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
> - LCP_DISC_REQ, CP_CODES};
> + CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
> + LCP_DISC_REQ, CP_CODES};

Do you think this looks better or worse after the change?

Andrew

2021-06-16 23:52:22

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 5/8] net: hdlc_ppp: fix the comments style issue

> static int cp_table[EVENTS][STATES] = {
> /* CLOSED STOPPED STOPPING REQ_SENT ACK_RECV ACK_SENT OPENED
> - 0 1 2 3 4 5 6 */
> + * 0 1 2 3 4 5 6
> + */
> {IRC|SCR|3, INV , INV , INV , INV , INV , INV }, /* START */
> { INV , 0 , 0 , 0 , 0 , 0 , 0 }, /* STOP */
> { INV , INV ,STR|2, SCR|3 ,SCR|3, SCR|5 , INV }, /* TO+ */

This probably reduces the readability of the code. So i would not make
this change.

Please remember these are only guidelines. Please don't blindly make
changes, or change it because some bot says so. Check that it actually
makes sense and the code is better afterwards.

Andrew

2021-06-17 02:43:39

by Lipeng

[permalink] [raw]
Subject: Re: [PATCH net-next 5/8] net: hdlc_ppp: fix the comments style issue


在 2021/6/17 0:20, Andrew Lunn 写道:
>> static int cp_table[EVENTS][STATES] = {
>> /* CLOSED STOPPED STOPPING REQ_SENT ACK_RECV ACK_SENT OPENED
>> - 0 1 2 3 4 5 6 */
>> + * 0 1 2 3 4 5 6
>> + */
>> {IRC|SCR|3, INV , INV , INV , INV , INV , INV }, /* START */
>> { INV , 0 , 0 , 0 , 0 , 0 , 0 }, /* STOP */
>> { INV , INV ,STR|2, SCR|3 ,SCR|3, SCR|5 , INV }, /* TO+ */
> This probably reduces the readability of the code. So i would not make
> this change.
>
> Please remember these are only guidelines. Please don't blindly make
> changes, or change it because some bot says so. Check that it actually
> makes sense and the code is better afterwards.
>
> Andrew
> .

Agree with you they are only guidelines.

The code should  follow the rules mostly, or  it will be diferent style
from different developer.

If that's the exception, i will drop this patch.


Thanks

            Peng Li

2021-06-17 02:44:49

by Lipeng

[permalink] [raw]
Subject: Re: [PATCH net-next 7/8] net: hdlc_ppp: remove redundant spaces


?? 2021/6/17 0:23, Andrew Lunn д??:
> On Wed, Jun 16, 2021 at 05:33:56PM +0800, Guangbin Huang wrote:
>> From: Peng Li <[email protected]>
>>
>> According to the chackpatch.pl,
>> no spaces is necessary at the start of a line.
>>
>> Signed-off-by: Peng Li <[email protected]>
>> Signed-off-by: Guangbin Huang <[email protected]>
>> ---
>> drivers/net/wan/hdlc_ppp.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
>> index 7b7c02d..53c668e 100644
>> --- a/drivers/net/wan/hdlc_ppp.c
>> +++ b/drivers/net/wan/hdlc_ppp.c
>> @@ -34,8 +34,8 @@
>>
>> enum {IDX_LCP = 0, IDX_IPCP, IDX_IPV6CP, IDX_COUNT};
>> enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
>> - CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
>> - LCP_DISC_REQ, CP_CODES};
>> + CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
>> + LCP_DISC_REQ, CP_CODES};
> Do you think this looks better or worse after the change?
>
> Andrew
> .

It is better after the change as the code style follow the same rule.

What's your suggestion?