Re: [PATCH v3] Staging: rtl8192e: rtllib_rx: fix alignment

[Philipp Hortmann <philipp.g.hortmann@gmail.com>]

On 6/26/24 14:56, Yusef Aslam wrote:
> From: Yusef Aslam <YUZi54780@outlook.com>
> Date: Wed, 26 Jun 2024 13:02:02 +0100
> Subject: [PATCH v3] Staging: rtl8192e: rtllib_rx: fix alignment
> 
> Fix alignment.
Hi Yusef,

The patch can be applied. Thanks for the change history.

But your patch description is not answering the why.

It could be: Fix alignment to increase readabiltiy.

For the first patch this is way to long and the indentations are really 
a mess. Issue is that lines then get to long which leads again to 
checkpatch issues. Please find some of my comments below.


> 
> Signed-off-by: Yusef Aslam <YUZi54780@outlook.com>
> ---
>   v3:
>   - Used the correct email addresses.
>   - Developed against the correct git repository.
>   v2:
>   - The email address of Greg Kroah-Hartman was wrong.
>   - Developed against the wrong git repository.
>   v1:
>   - Developed against the wrong git repository.
> 
>   drivers/staging/rtl8192e/rtllib_rx.c | 110 +++++++++++++--------------
>   1 file changed, 55 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
> index 84ca5d769b7e..1f6c4a3de5c2 100644
> --- a/drivers/staging/rtl8192e/rtllib_rx.c
> +++ b/drivers/staging/rtl8192e/rtllib_rx.c
> @@ -410,7 +410,7 @@ static bool add_reorder_entry(struct rx_ts_record *ts,
>   	while (list->next != &ts->rx_pending_pkt_list) {
>   		if (SN_LESS(pReorderEntry->SeqNum, ((struct rx_reorder_entry *)
>   		    list_entry(list->next, struct rx_reorder_entry,
> -		    list))->SeqNum))
> +			       list))->SeqNum))
>   			list = list->next;
>   		else if (SN_EQUAL(pReorderEntry->SeqNum,
>   			((struct rx_reorder_entry *)list_entry(list->next,
> @@ -736,7 +736,7 @@ static u8 parse_subframe(struct rtllib_device *ieee, struct sk_buff *skb,
>   	/* just for debug purpose */
>   	SeqNum = WLAN_GET_SEQ_SEQ(le16_to_cpu(hdr->seq_ctrl));
>   	if ((RTLLIB_QOS_HAS_SEQ(fc)) &&
> -	   (((union frameqos *)(skb->data + RTLLIB_3ADDR_LEN))->field.reserved))
> +	    (((union frameqos *)(skb->data + RTLLIB_3ADDR_LEN))->field.reserved))
>   		is_aggregate_frame = true;
>   
>   	if (RTLLIB_QOS_HAS_SEQ(fc))
> @@ -876,13 +876,13 @@ static int rtllib_rx_check_duplicate(struct rtllib_device *ieee,
>   	frag = WLAN_GET_SEQ_FRAG(sc);
>   
>   	if (!ieee->ht_info->cur_rx_reorder_enable ||
> -		!ieee->current_network.qos_data.active ||
> -		!is_data_frame(skb->data) ||
> -		is_legacy_data_frame(skb->data)) {
> -		if (!ieee80211_is_beacon(hdr->frame_control)) {
> -			if (is_duplicate_packet(ieee, hdr))
> -				return -1;
> -		}
> +	    !ieee->current_network.qos_data.active ||
> +	    !is_data_frame(skb->data) ||
> +	    is_legacy_data_frame(skb->data)) {
> +	  if (!ieee80211_is_beacon(hdr->frame_control)) {
Why is the above line indentation 1 Tab two spaces?
> +	    if (is_duplicate_packet(ieee, hdr))
> +	      return -1;
> +	  }
>   	} else {
>   		struct rx_ts_record *ts = NULL;
>   
> @@ -976,7 +976,7 @@ static int rtllib_rx_data_filter(struct rtllib_device *ieee, struct ieee80211_hd
>   }
>   
>   static int rtllib_rx_get_crypt(struct rtllib_device *ieee, struct sk_buff *skb,
> -			struct lib80211_crypt_data **crypt, size_t hdrlen)
> +			       struct lib80211_crypt_data **crypt, size_t hdrlen)
>   {
>   	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>   	u16 fc = le16_to_cpu(hdr->frame_control);
> @@ -1008,8 +1008,8 @@ static int rtllib_rx_get_crypt(struct rtllib_device *ieee, struct sk_buff *skb,
>   }
>   
>   static int rtllib_rx_decrypt(struct rtllib_device *ieee, struct sk_buff *skb,
> -		      struct rtllib_rx_stats *rx_stats,
> -		      struct lib80211_crypt_data *crypt, size_t hdrlen)
> +			     struct rtllib_rx_stats *rx_stats,
> +			     struct lib80211_crypt_data *crypt, size_t hdrlen)
>   {
>   	struct ieee80211_hdr *hdr;
>   	int keyidx = 0;
> @@ -1092,9 +1092,9 @@ static int rtllib_rx_decrypt(struct rtllib_device *ieee, struct sk_buff *skb,
>   	 * encrypted/authenticated
>   	 */
>   	if ((fc & IEEE80211_FCTL_PROTECTED) &&
> -		rtllib_rx_frame_decrypt_msdu(ieee, skb, keyidx, crypt)) {
> -		netdev_info(ieee->dev, "%s: ==>decrypt msdu error\n", __func__);
> -		return -1;
> +	    rtllib_rx_frame_decrypt_msdu(ieee, skb, keyidx, crypt)) {
> +	  netdev_info(ieee->dev, "%s: ==>decrypt msdu error\n", __func__);
> +	  return -1;
Why are the two above lines indentation 1 Tab two spaces?
>   	}
>   
>   	hdr = (struct ieee80211_hdr *)skb->data;
> @@ -1152,10 +1152,10 @@ static void rtllib_rx_check_leave_lps(struct rtllib_device *ieee, u8 unicast,
>   }
>   
>   static void rtllib_rx_indicate_pkt_legacy(struct rtllib_device *ieee,
> -		struct rtllib_rx_stats *rx_stats,
> -		struct rtllib_rxb *rxb,
> -		u8 *dst,
> -		u8 *src)
> +					  struct rtllib_rx_stats *rx_stats,
> +					  struct rtllib_rxb *rxb,
> +					  u8 *dst,
> +					  u8 *src)
>   {
>   	struct net_device *dev = ieee->dev;
>   	u16 ethertype;
> @@ -1175,17 +1175,17 @@ static void rtllib_rx_indicate_pkt_legacy(struct rtllib_device *ieee,
>   			 */
>   			ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
>   			if (sub_skb->len >= 8 &&
> -				((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
> -				ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
> -				memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
> -				/* remove RFC1042 or Bridge-Tunnel encapsulation
> -				 * and replace EtherType
> -				 */
> -				skb_pull(sub_skb, SNAP_SIZE);
> -				ether_addr_copy(skb_push(sub_skb, ETH_ALEN),
> -						src);
> -				ether_addr_copy(skb_push(sub_skb, ETH_ALEN),
> -						dst);
> +			    ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
> +			      ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
> +			     memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
> +			  /* remove RFC1042 or Bridge-Tunnel encapsulation
> +			   * and replace EtherType
> +			   */
> +			  skb_pull(sub_skb, SNAP_SIZE);
> +			  ether_addr_copy(skb_push(sub_skb, ETH_ALEN),
> +					  src);
> +			  ether_addr_copy(skb_push(sub_skb, ETH_ALEN),
> +					  dst);
Why are the eight above lines indentation 3 Tab two spaces?
>   			} else {
>   				u16 len;
>   				/* Leave Ethernet header part of hdr
> @@ -1220,7 +1220,7 @@ static void rtllib_rx_indicate_pkt_legacy(struct rtllib_device *ieee,
>   }
>   
>   static int rtllib_rx_infra_adhoc(struct rtllib_device *ieee, struct sk_buff *skb,
> -		 struct rtllib_rx_stats *rx_stats)
> +				 struct rtllib_rx_stats *rx_stats)
>   {
>   	struct net_device *dev = ieee->dev;
>   	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> @@ -1323,9 +1323,9 @@ static int rtllib_rx_infra_adhoc(struct rtllib_device *ieee, struct sk_buff *skb
>   		TID = frame_qos_tid(skb->data);
>   		SeqNum = WLAN_GET_SEQ_SEQ(sc);
>   		rtllib_get_ts(ieee, (struct ts_common_info **)&ts, hdr->addr2, TID,
> -		      RX_DIR, true);
> +			      RX_DIR, true);
>   		if (TID != 0 && TID != 3)
> -			ieee->bis_any_nonbepkts = true;
> +		  ieee->bis_any_nonbepkts = true;
Why is the above line indentation 2 Tab two spaces?
I am stopping here sorry.

Bye Philipp
>   	}
>   
>   	/* Parse rx data frame (For AMSDU) */
> @@ -1380,7 +1380,7 @@ static int rtllib_rx_infra_adhoc(struct rtllib_device *ieee, struct sk_buff *skb
>   }
>   
>   static int rtllib_rx_monitor(struct rtllib_device *ieee, struct sk_buff *skb,
> -		 struct rtllib_rx_stats *rx_stats)
> +			     struct rtllib_rx_stats *rx_stats)
>   {
>   	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>   	u16 fc = le16_to_cpu(hdr->frame_control);
> @@ -1412,7 +1412,7 @@ static int rtllib_rx_monitor(struct rtllib_device *ieee, struct sk_buff *skb,
>    * This function is called only as a tasklet (software IRQ).
>    */
>   int rtllib_rx(struct rtllib_device *ieee, struct sk_buff *skb,
> -		 struct rtllib_rx_stats *rx_stats)
> +	      struct rtllib_rx_stats *rx_stats)
>   {
>   	int ret = 0;
>   
> @@ -1577,10 +1577,10 @@ static int rtllib_parse_qos_info_param_IE(struct rtllib_device *ieee,
>   		struct rtllib_qos_parameter_info param_element;
>   
>   		rc = rtllib_read_qos_param_element(&param_element,
> -						      info_element);
> +						   info_element);
>   		if (rc == 0) {
>   			rtllib_qos_convert_ac_to_parameters(&param_element,
> -							       &(network->qos_data));
> +							    &(network->qos_data));
>   			network->flags |= NETWORK_HAS_QOS_PARAMETERS;
>   			network->qos_data.param_count =
>   			    param_element.info_element.ac_info & 0x0F;
> @@ -1864,7 +1864,7 @@ static void rtllib_parse_mfie_ht_cap(struct rtllib_info_element *info_element,
>   	if (*tmp_htcap_len != 0) {
>   		ht->bd_ht_spec_ver = HT_SPEC_VER_EWC;
>   		ht->bd_ht_cap_len = min_t(u16, *tmp_htcap_len,
> -				       sizeof(ht->bd_ht_cap_buf));
> +					  sizeof(ht->bd_ht_cap_buf));
>   		memcpy(ht->bd_ht_cap_buf, info_element->data, ht->bd_ht_cap_len);
>   
>   		ht->bd_support_ht = true;
> @@ -1882,10 +1882,10 @@ static void rtllib_parse_mfie_ht_cap(struct rtllib_info_element *info_element,
>   }
>   
>   int rtllib_parse_info_param(struct rtllib_device *ieee,
> -		struct rtllib_info_element *info_element,
> -		u16 length,
> -		struct rtllib_network *network,
> -		struct rtllib_rx_stats *stats)
> +			    struct rtllib_info_element *info_element,
> +			    u16 length,
> +			    struct rtllib_network *network,
> +			    struct rtllib_rx_stats *stats)
>   {
>   	u8 i;
>   	short offset;
> @@ -2329,10 +2329,10 @@ static inline void update_network(struct rtllib_device *ieee,
>   
>   	dst->wmm_info = src->wmm_info;
>   	if (src->wmm_param[0].ac_aci_acm_aifsn ||
> -	   src->wmm_param[1].ac_aci_acm_aifsn ||
> -	   src->wmm_param[2].ac_aci_acm_aifsn ||
> -	   src->wmm_param[3].ac_aci_acm_aifsn)
> -		memcpy(dst->wmm_param, src->wmm_param, WME_AC_PRAM_LEN);
> +	    src->wmm_param[1].ac_aci_acm_aifsn ||
> +	    src->wmm_param[2].ac_aci_acm_aifsn ||
> +	    src->wmm_param[3].ac_aci_acm_aifsn)
> +	  memcpy(dst->wmm_param, src->wmm_param, WME_AC_PRAM_LEN);
>   
>   	dst->SignalStrength = src->SignalStrength;
>   	dst->RSSI = src->RSSI;
> @@ -2450,7 +2450,7 @@ static inline void rtllib_process_probe_response(
>   
>   	spin_lock_irqsave(&ieee->lock, flags);
>   	if (is_same_network(&ieee->current_network, network,
> -	   (network->ssid_len ? 1 : 0))) {
> +			    (network->ssid_len ? 1 : 0))) {
>   		update_network(ieee, &ieee->current_network, network);
>   		if ((ieee->current_network.mode == WIRELESS_MODE_N_24G ||
>   		     ieee->current_network.mode == WIRELESS_MODE_G) &&
> @@ -2467,10 +2467,10 @@ static inline void rtllib_process_probe_response(
>   	}
>   	list_for_each_entry(target, &ieee->network_list, list) {
>   		if (is_same_network(target, network,
> -		   (target->ssid_len ? 1 : 0)))
> -			break;
> +				    (target->ssid_len ? 1 : 0)))
> +		  break;
>   		if (!oldest || (target->last_scanned < oldest->last_scanned))
> -			oldest = target;
> +		  oldest = target;
>   	}
>   
>   	/* If we didn't find a match, then get a new network slot to initialize
> @@ -2528,9 +2528,9 @@ static inline void rtllib_process_probe_response(
>   	spin_unlock_irqrestore(&ieee->lock, flags);
>   	if (ieee80211_is_beacon(frame_ctl) &&
>   	    is_same_network(&ieee->current_network, network,
> -	    (network->ssid_len ? 1 : 0)) &&
> +			    (network->ssid_len ? 1 : 0)) &&
>   	    (ieee->link_state == MAC80211_LINKED)) {
> -		ieee->handle_beacon(ieee->dev, beacon, &ieee->current_network);
> +	  ieee->handle_beacon(ieee->dev, beacon, &ieee->current_network);
>   	}
>   free_network:
>   	kfree(network);
> @@ -2553,9 +2553,9 @@ static void rtllib_rx_mgt(struct rtllib_device *ieee,
>   				stats);
>   
>   		if (ieee->sta_sleep || (ieee->ps != RTLLIB_PS_DISABLED &&
> -		    ieee->iw_mode == IW_MODE_INFRA &&
> -		    ieee->link_state == MAC80211_LINKED))
> -			schedule_work(&ieee->ps_task);
> +					ieee->iw_mode == IW_MODE_INFRA &&
> +					ieee->link_state == MAC80211_LINKED))
> +		  schedule_work(&ieee->ps_task);
>   	} else if (ieee80211_is_probe_resp(header->frame_control)) {
>   		netdev_dbg(ieee->dev, "received PROBE RESPONSE\n");
>   		rtllib_process_probe_response(ieee, (struct rtllib_probe_response *)header,