Re: [PATCH 4/4] scan: Separate cck rates attr creation into logical block

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

Hi Marcel,

>> +	if (L_WARN_ON(!(supported = wiphy_get_supported_rates(sc->wiphy,
>> +							NL80211_BAND_2GHZ,
>> +							&num_supported))))
>> +		return;
> 
> these kind of “nested” statements become really hard to read and check if they are correct.
> 
> 	supported = wiphy_get_supported_rates(sc->wiphy, ..);
> 	if (L_WARN_ON(!supported))
> 		return;
> 
> I realize that this code is the same as below, but lets improve readability as well. It is fine to submit a cleanup patch to improve readability if we find such places.

Actually, I explicitly requested that this be the way it is.  For 
L_WARN_ON, I want the resultant warning to carry as much info as 
possible.  Having a message like "Warning: !supported" show up is pretty 
pointless.  Yes it carries the line number and filename, but it still 
requires an extra step.  And things can change between different 
versions of iwd, etc.

So while the readability is a little worse, I think the hit is worth it 
for cases where L_WARN_ON is used.

Regards,
-Denis