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: 1521 bytes --]

On Tue, Oct 06, 2015 at 12:32:20AM -0700, Frank Rowand wrote:
> On 10/5/2015 9:10 PM, David Gibson wrote:
> > 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.
> > 
> 
> Checking !pos first results in either an early return, a goto,
> or more deeply nesting the

Early return is fine by me.

-- 
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 --]