Re: [PATCH V3] brcmfmac: allow NVRAM values to contain spaces

[Arend van Spriel <arend@broadcom.com>]

On 05/22/15 23:45, Rafał Miłecki wrote:
> Platform NVRAMs often contain values with spaces. Even if right now most
> firmware-supported entries are simple values, we shouldn't reject these
> with spaces. It was semi-confirmed by Broadocm in the early patch adding
> support for platform NVRAMs.

Well, here is my full confirmation ;-) and thanks.

Acked-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Rafał Miłecki<zajec5@gmail.com>
> ---
> V2: Minor change in commit message only. Provide an example of NVRAM entry
>      using space (not just a '#'). It's unprefixed (shouldn't be uploaded to
>      hardware anyway), but proves it's allowed in general.
> V3: As commented by Arend, accept space only (not a '#').
> ---
>   drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
> index 45d7191..baad939 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
> @@ -66,6 +66,12 @@ struct nvram_parser {
>   	bool multi_dev_v2;
>   };
>
> +/**
> + * is_nvram_char() - check if char is a valid one for NVRAM entry
> + *
> + * It accepts all printable ASCII chars except for '#' which opens a comment.
> + * Please note that ' ' (space) while accepted is not a valid key name char.
> + */
>   static bool is_nvram_char(char c)
>   {
>   	/* comment marker excluded */
> @@ -73,7 +79,7 @@ static bool is_nvram_char(char c)
>   		return false;
>
>   	/* key and value may have any other readable character */
> -	return (c>  0x20&&  c<  0x7f);
> +	return (c>= 0x20&&  c<  0x7f);
>   }
>
>   static bool is_whitespace(char c)
> @@ -120,7 +126,7 @@ static enum nvram_parser_state brcmf_nvram_handle_key(struct nvram_parser *nvp)
>   			nvp->multi_dev_v1 = true;
>   		if (strncmp(&nvp->fwnv->data[nvp->entry], "pcie/", 5) == 0)
>   			nvp->multi_dev_v2 = true;
> -	} else if (!is_nvram_char(c)) {
> +	} else if (!is_nvram_char(c) || c == ' ') {
>   		brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n",
>   			  nvp->line, nvp->column);
>   		return COMMENT;