On Fri, 7 Jun 2024 19:01:27 +0100, Simon Horman wrote:

> Hi Abhinav,
>
> I suspect this will now only report a failure if tail fails,
> but ignore ethtool failures.

Hi Simon,

I agree, I missed this part earlier. After taking other suggestion
into account, we don't need this tail and I have removed it.

> Shellcheck warns that the above reads words rather than lines,
> and recommends using read instead.
>
> I think that is ok, because the construction reduces lines to single words.
> But it does seem a bit awkward to call grep, awk and sed for this.
>
> I wonder if the following construction nicer:
>
> while read -r FEATURE VALUE FIXED; do
>       [ "$FEAT" != "Features" ] || continue # Skip "Features" line
>       [ "$FIXED" != "[fixed]" ] || continue # Skip fixed features
>       feature="${FEATURE%:*}"
>       ...
> done < "$TMP_ETHTOOL_FEATURES"

I have re-submitted a v2 of patch here keeping the above change:
https://lore.kernel.org/all/[email protected]/

Please review. Thank you. 

Reply via email to