| 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
| ||
|
Message-ID: <20220405001019.GA1200980@alison-desk>
Date: Mon, 4 Apr 2022 17:10:19 -0700
From: Alison Schofield <alison.schofield@...el.com>
To: Alaa Mohamed <eng.alaamohamedsoliman.am@...il.com>
Cc: "outreachy@...ts.linux.dev" <outreachy@...ts.linux.dev>,
"Larry.Finger@...inger.net" <Larry.Finger@...inger.net>,
"phil@...lpotter.co.uk" <phil@...lpotter.co.uk>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] staging: r8188eu: add space around arithmatic sign
On Mon, Apr 04, 2022 at 07:39:16AM -0700, Alaa Mohamed wrote:
> Reported by checkpatch:
>
> CHECK: spaces preferred around that '+' , '-' and '*'
>
> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@...il.com>
Hi Alaa,
I see you already picked up an ACK on this!
I'm going to comment on the format since I'm aware this is your
first patchset! The grouping is good - 2 cleanups in same file.
Please search in the tutorial for further info on how to present these
as a patchset. You can also peruse the outreachy list for formats of
patchsets as well. I'd expect to see a cover letter.
The other thing I noticed was this:
>> [PATCH 1/2] staging: r8188eu: add space around arithmatic sign
>> [PATCH 2/2] staging: r8188eu: Add line after declarations
to which I was going to say - 'Well which is the actual preference
for this file or driver: 'add' or 'Add', ie. uppercase or lowercase?
Looking at the history, it's mixed. So, I guess I'll just suggest
you don't mix it within a patchset. If a maintainer cares, and try
to set a pattern for the file or driver..
spelling fixup please:
s/arithmatic/arithmetic
and perhpas this would be clearer:
s/sign/operator (They are arithmetic operators, not signs.)
Alison
> ---
> drivers/staging/r8188eu/core/rtw_br_ext.c | 58 +++++++++++------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index d68611ef22f8..7c664f80fa99 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -87,19 +87,19 @@ static int skb_pull_and_merge(struct sk_buff *skb, unsigned char *src, int len)
> int tail_len;
> unsigned long end, tail;
>
> - if ((src+len) > skb_tail_pointer(skb) || skb->len < len)
> + if ((src + len) > skb_tail_pointer(skb) || skb->len < len)
> return -1;
>
> tail = (unsigned long)skb_tail_pointer(skb);
> - end = (unsigned long)src+len;
> + end = (unsigned long)src + len;
> if (tail < end)
> return -1;
>
> - tail_len = (int)(tail-end);
> + tail_len = (int)(tail - end);
> if (tail_len > 0)
> - memmove(src, src+len, tail_len);
> + memmove(src, src + len, tail_len);
>
> - skb_trim(skb, skb->len-len);
> + skb_trim(skb, skb->len - len);
> return 0;
> }
>
> @@ -117,7 +117,7 @@ static void __nat25_generate_ipv4_network_addr(unsigned char *networkAddr,
> memset(networkAddr, 0, MAX_NETWORK_ADDR_LEN);
>
> networkAddr[0] = NAT25_IPV4;
> - memcpy(networkAddr+7, (unsigned char *)ipAddr, 4);
> + memcpy(networkAddr + 7, (unsigned char *)ipAddr, 4);
> }
>
> static void __nat25_generate_pppoe_network_addr(unsigned char *networkAddr,
> @@ -126,8 +126,8 @@ static void __nat25_generate_pppoe_network_addr(unsigned char *networkAddr,
> memset(networkAddr, 0, MAX_NETWORK_ADDR_LEN);
>
> networkAddr[0] = NAT25_PPPOE;
> - memcpy(networkAddr+1, (unsigned char *)sid, 2);
> - memcpy(networkAddr+3, (unsigned char *)ac_mac, 6);
> + memcpy(networkAddr + 1, (unsigned char *)sid, 2);
> + memcpy(networkAddr + 3, (unsigned char *)ac_mac, 6);
> }
>
> static void __nat25_generate_ipv6_network_addr(unsigned char *networkAddr,
> @@ -136,17 +136,17 @@ static void __nat25_generate_ipv6_network_addr(unsigned char *networkAddr,
> memset(networkAddr, 0, MAX_NETWORK_ADDR_LEN);
>
> networkAddr[0] = NAT25_IPV6;
> - memcpy(networkAddr+1, (unsigned char *)ipAddr, 16);
> + memcpy(networkAddr + 1, (unsigned char *)ipAddr, 16);
> }
>
> static unsigned char *scan_tlv(unsigned char *data, int len, unsigned char tag, unsigned char len8b)
> {
> while (len > 0) {
> - if (*data == tag && *(data+1) == len8b && len >= len8b*8)
> - return data+2;
> + if (*data == tag && *(data + 1) == len8b && len >= len8b * 8)
> + return data + 2;
>
> - len -= (*(data+1))*8;
> - data += (*(data+1))*8;
> + len -= (*(data + 1)) * 8;
> + data += (*(data + 1)) * 8;
> }
> return NULL;
> }
> @@ -158,7 +158,7 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char
>
> if (icmphdr->icmp6_type == NDISC_ROUTER_SOLICITATION) {
> if (len >= 8) {
> - mac = scan_tlv(&data[8], len-8, 1, 1);
> + mac = scan_tlv(&data[8], len - 8, 1, 1);
> if (mac) {
> memcpy(mac, replace_mac, 6);
> return 1;
> @@ -166,7 +166,7 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char
> }
> } else if (icmphdr->icmp6_type == NDISC_ROUTER_ADVERTISEMENT) {
> if (len >= 16) {
> - mac = scan_tlv(&data[16], len-16, 1, 1);
> + mac = scan_tlv(&data[16], len - 16, 1, 1);
> if (mac) {
> memcpy(mac, replace_mac, 6);
> return 1;
> @@ -174,7 +174,7 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char
> }
> } else if (icmphdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
> if (len >= 24) {
> - mac = scan_tlv(&data[24], len-24, 1, 1);
> + mac = scan_tlv(&data[24], len - 24, 1, 1);
> if (mac) {
> memcpy(mac, replace_mac, 6);
> return 1;
> @@ -182,7 +182,7 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char
> }
> } else if (icmphdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT) {
> if (len >= 24) {
> - mac = scan_tlv(&data[24], len-24, 2, 1);
> + mac = scan_tlv(&data[24], len - 24, 2, 1);
> if (mac) {
> memcpy(mac, replace_mac, 6);
> return 1;
> @@ -190,7 +190,7 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char
> }
> } else if (icmphdr->icmp6_type == NDISC_REDIRECT) {
> if (len >= 40) {
> - mac = scan_tlv(&data[40], len-40, 2, 1);
> + mac = scan_tlv(&data[40], len - 40, 2, 1);
> if (mac) {
> memcpy(mac, replace_mac, 6);
> return 1;
> @@ -396,7 +396,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> tmp = be32_to_cpu(iph->saddr);
> __nat25_generate_ipv4_network_addr(networkAddr, &tmp);
> /* record source IP address and , source mac address into db */
> - __nat25_db_network_insert(priv, skb->data+ETH_ALEN, networkAddr);
> + __nat25_db_network_insert(priv, skb->data + ETH_ALEN, networkAddr);
> return 0;
> default:
> return -1;
> @@ -421,7 +421,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> arp_ptr += arp->ar_hln;
> sender = (unsigned int *)arp_ptr;
> __nat25_generate_ipv4_network_addr(networkAddr, sender);
> - __nat25_db_network_insert(priv, skb->data+ETH_ALEN, networkAddr);
> + __nat25_db_network_insert(priv, skb->data + ETH_ALEN, networkAddr);
> return 0;
> default:
> return -1;
> @@ -458,22 +458,22 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> sizeof(tag_buf))
> return -1;
>
> - memcpy(tag->tag_data+MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN,
> + memcpy(tag->tag_data + MAGIC_CODE_LEN + RTL_RELAY_TAG_LEN,
> pOldTag->tag_data, old_tag_len);
>
> - if (skb_pull_and_merge(skb, (unsigned char *)pOldTag, TAG_HDR_LEN+old_tag_len) < 0)
> + if (skb_pull_and_merge(skb, (unsigned char *)pOldTag, TAG_HDR_LEN + old_tag_len) < 0)
> return -1;
>
> - ph->length = htons(ntohs(ph->length)-TAG_HDR_LEN-old_tag_len);
> + ph->length = htons(ntohs(ph->length) - TAG_HDR_LEN - old_tag_len);
> }
>
> tag->tag_type = PTT_RELAY_SID;
> - tag->tag_len = htons(MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN+old_tag_len);
> + tag->tag_len = htons(MAGIC_CODE_LEN + RTL_RELAY_TAG_LEN + old_tag_len);
>
> /* insert the magic_code+client mac in relay tag */
> pMagic = (unsigned short *)tag->tag_data;
> *pMagic = htons(MAGIC_CODE);
> - memcpy(tag->tag_data+MAGIC_CODE_LEN, skb->data+ETH_ALEN, ETH_ALEN);
> + memcpy(tag->tag_data + MAGIC_CODE_LEN, skb->data + ETH_ALEN, ETH_ALEN);
>
> /* Add relay tag */
> if (__nat25_add_pppoe_tag(skb, tag) < 0)
> @@ -486,7 +486,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> return -2;
>
> if (priv->pppoe_connection_in_progress == 0)
> - memcpy(priv->pppoe_addr, skb->data+ETH_ALEN, ETH_ALEN);
> + memcpy(priv->pppoe_addr, skb->data + ETH_ALEN, ETH_ALEN);
>
> priv->pppoe_connection_in_progress = WAIT_TIME_PPPOE;
> }
> @@ -496,11 +496,11 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> } else { /* session phase */
> __nat25_generate_pppoe_network_addr(networkAddr, skb->data, &ph->sid);
>
> - __nat25_db_network_insert(priv, skb->data+ETH_ALEN, networkAddr);
> + __nat25_db_network_insert(priv, skb->data + ETH_ALEN, networkAddr);
>
> if (!priv->ethBrExtInfo.addPPPoETag &&
> priv->pppoe_connection_in_progress &&
> - !memcmp(skb->data+ETH_ALEN, priv->pppoe_addr, ETH_ALEN))
> + !memcmp(skb->data + ETH_ALEN, priv->pppoe_addr, ETH_ALEN))
> priv->pppoe_connection_in_progress = 0;
> }
> return 0;
> @@ -548,7 +548,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> case NAT25_INSERT:
> if (memcmp(&iph->saddr, "\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0", 16)) {
> __nat25_generate_ipv6_network_addr(networkAddr, (unsigned int *)&iph->saddr);
> - __nat25_db_network_insert(priv, skb->data+ETH_ALEN, networkAddr);
> + __nat25_db_network_insert(priv, skb->data + ETH_ALEN, networkAddr);
>
> if (iph->nexthdr == IPPROTO_ICMPV6 &&
> skb->len > (ETH_HLEN + sizeof(*iph) + 4)) {
> --
> 2.35.1
>
>
Powered by blists - more mailing lists