[U-Boot,3/3] ti: common: board_detect: Return a valid empty string for un-initialized eeprom

Current logic for query of revision, board_name, config returns
NULL. Users of these functions do a direct strncmp to compare.
Unfortunately, as per conventions require two valid strings to compare
against and the current implementation causes a crash when compared
with NULL.

We'd still like to maintain the simplistic usage of these APIs instead
of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
the place.

Hence, since the version, name and config is already pre-initialized
with empty string, just dont check for invalid header in the first
place and return the empty string to the caller.

Reported-by: Brad Griffis <bgriffis@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 board/ti/common/board_detect.c | 12 +++---------
 board/ti/common/board_detect.h |  6 +++---
 2 files changed, 6 insertions(+), 12 deletions(-)

On Tuesday 11 October 2016 11:09 PM, Nishanth Menon wrote:
> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>


Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh


> ---
>  board/ti/common/board_detect.c | 12 +++---------
>  board/ti/common/board_detect.h |  6 +++---
>  2 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index 13aac2f03037..6e7ca9196d22 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -231,9 +231,7 @@ char * __maybe_unused board_ti_get_rev(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->version;
>  }
>  
> @@ -241,9 +239,7 @@ char * __maybe_unused board_ti_get_config(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->config;
>  }
>  
> @@ -251,9 +247,7 @@ char * __maybe_unused board_ti_get_name(void)
>  {
>  	struct ti_common_eeprom *ep = TI_EEPROM_DATA;
>  
> -	if (ep->header == TI_DEAD_EEPROM_MAGIC)
> -		return NULL;
> -
> +	/* if ep->header == TI_DEAD_EEPROM_MAGIC, this is empty already */
>  	return ep->name;
>  }
>  
> diff --git a/board/ti/common/board_detect.h b/board/ti/common/board_detect.h
> index eb17f6f52a12..2ded3ed7764b 100644
> --- a/board/ti/common/board_detect.h
> +++ b/board/ti/common/board_detect.h
> @@ -141,7 +141,7 @@ bool board_ti_rev_is(char *rev_tag, int cmp_len);
>  /**
>   * board_ti_get_rev() - Get board revision for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board revision otherwise
>   */
>  char *board_ti_get_rev(void);
> @@ -149,7 +149,7 @@ char *board_ti_get_rev(void);
>  /**
>   * board_ti_get_config() - Get board config for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board config otherwise
>   */
>  char *board_ti_get_config(void);
> @@ -157,7 +157,7 @@ char *board_ti_get_config(void);
>  /**
>   * board_ti_get_name() - Get board name for TI EVMs
>   *
> - * Return: NULL if eeprom was'nt read.
> + * Return: Empty string if eeprom was'nt read.
>   *	   Board name otherwise
>   */
>  char *board_ti_get_name(void);
>

On Tue, Oct 11, 2016 at 12:39:05PM -0500, Nishanth Menon wrote:

> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

On Tue, Oct 11, 2016 at 12:39:05PM -0500, Nishanth Menon wrote:

> Current logic for query of revision, board_name, config returns
> NULL. Users of these functions do a direct strncmp to compare.
> Unfortunately, as per conventions require two valid strings to compare
> against and the current implementation causes a crash when compared
> with NULL.
> 
> We'd still like to maintain the simplistic usage of these APIs instead
> of redundant if (string) res=strncmp(fn(),"cmp",n); flowing all over
> the place.
> 
> Hence, since the version, name and config is already pre-initialized
> with empty string, just dont check for invalid header in the first
> place and return the empty string to the caller.
> 
> Reported-by: Brad Griffis <bgriffis@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master (before v2016.11-rc3), thanks!