Re: [PATCH 13/30] brcmfmac: Clarify if using braces.

On 30/08/17 20:11, Arend van Spriel wrote:

quoted

Whilst this if () statement is technically correct, it lacks clarity.

I don't see the unclarity here. In my opinion people reading the code
should have a good level in C language and a decent level of curiosity
when they come across a function/macro like skb_queue_walk().

I thought it to be part of the general codingstyle for the kernel that
multi-line ifs and elses should be in braces, although I accept that
technically the if-clause is a single block-level statement.

Having said that, *this* specific example falls into a grey area in the
codingstyle, which covers multi-statement, multi-line if() clauses and
single-statement, single-line ones. It does not cover single-statement,
multi-line examples such as the one here.

Whilst I can't therefore definitively justify my position, I can show,
for example, line 999 in net/mac80211/iface.c where a for() statement
uses braces around the skb_queue_walk()

for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
        skb_queue_walk_safe(&local->pending[i], skb, tmp) {
                struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
                if (info->control.vif == &sdata->vif) {
                        __skb_unlink(skb, &local->pending[i]);
                        ieee80211_free_txskb(&local->hw, skb);
                }
        }
}


And the following in ath9k_htc_tx_cleanup_queue()

if (process) {
        skb_queue_walk_safe(&queue, skb, tmp) {
                __skb_unlink(skb, &queue);
                ath9k_htc_tx_process(priv, skb, NULL);
        }
}

So I feel that we should do the same.

-Ian