Re: [RFC PATCH v6 1/3] dtc: protect against null pointer dereference in srcpos_string()

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Fri, Oct 02, 2015 at 09:49:08PM -0700, Frank Rowand wrote:
> From: Frank Rowand <frank.rowand-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> 
> Check for NULL pos before dereferencing it in srcpos_string().
> 
> Signed-off-by: Frank Rowand <frank.rowand-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> ---
>  srcpos.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: b/srcpos.c
> ===================================================================
> --- a/srcpos.c
> +++ b/srcpos.c
> @@ -268,11 +268,13 @@ srcpos_string(struct srcpos *pos)
>  	char *pos_str;
>  	int rc;
>  
> -	if (pos)
> +	if (pos && pos->file)
>  		fname = pos->file->name;
>  
>  
> -	if (pos->first_line != pos->last_line)
> +	if (!pos)
> +		rc = asprintf(&pos_str, "%s:<no-line>", fname);
> +	else if (pos->first_line != pos->last_line)

This logic still seems backwards to me.  I'd really prefer the !pos
check to go first, then !pos->file, then the normal case.

>  		rc = asprintf(&pos_str, "%s:%d.%d-%d.%d", fname,
>  			      pos->first_line, pos->first_column,
>  			      pos->last_line, pos->last_column);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]