Re: [PATCH 3/8] Enhance source position implementation.

[Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]

Jon Loeliger wrote:
> @@ -106,7 +123,9 @@ fail:
>  	die("Couldn't open \"%s\": %s\n", fname, strerror(errno));
>  }
>  
> -void dtc_close_file(struct dtc_file *file)
> +
> +void
> +dtc_close_file(struct dtc_file *file)

:-(

> +char *
> +srcpos_string(srcpos *pos)
> +{
> +#	define POS_BUF_SIZE	(100)

Local #defines make $YOUTHFUL_DEITY_OFFSPRING cry (and the whitespace 
after the # doesn't help).  Declare a "const int", or just use sizeof(buf).

> +	const char *fname;
> +	char buf[POS_BUF_SIZE];
> +
> +	if (pos->file && pos->file->name)
> +		fname = pos->file->name;
> +	else
> +		fname = "<no-file>";
> +
> +	if (pos->first_line == pos->last_line) {
> +		if (pos->first_column == pos->last_column) {
> +			snprintf(buf, POS_BUF_SIZE, "%s %d:%d",
> +				 fname, pos->first_line, pos->first_column);
> +		} else {
> +			snprintf(buf, POS_BUF_SIZE, "%s %d:%d-%d",
> +				 fname, pos->first_line,
> +				 pos->first_column, pos->last_column);
> +		}
> +
> +	} else {
> +		snprintf(buf, POS_BUF_SIZE, "%s %d:%d - %d:%d",
> +			 fname,
> +			 pos->first_line, pos->first_column,
> +			 pos->last_line, pos->last_column);
> +	}

Seems a little elaborate; are there cases where reporting the 
line/column of the first character of the token is not clear?

> +	return strdup(buf);

Why not just dynamically allocate the buffer in the first place?  Or if 
you care about the memory wastage (which you probably don't, given that 
you just leak the allocated string in the callers in later patches), 
make this function take a FILE * to output to, rather than return an 
allocated string.

-Scott