[ccan] [PATCH] avoid segfault when module_builds' linking fails

[Rusty Russell]

pb-ccan at barker.dropbear.id.au writes:
> From: Peter Barker <pb-ccan at barker.dropbear.id.au>
>
> In the case that the objects built but linking failed, module_builds.c
> called score_file_error with a NULL ccan_file object and 0 for line
> number.
>
> score_file_error assumed that the ccan_file object it is passed was
> not-NULL when appending file errors to the score's aggregate error
> string.  It attempted to dereference it to get "fullname".
>
> score_error was factored out from score_file_error.  It takes a
> "source" parameter, which is the file's full name (and possibly line
> number) in the score_file_error case, and the ccan module name in the
> case of link failure.

Applied (caught this in time before I pushed).  I put a "ccanlint:"
prefix in the subject.

Thanks!
Rusty.

> ---
>  tools/ccanlint/ccanlint.h            |    4 +++
>  tools/ccanlint/file_analysis.c       |   46 +++++++++++++++++++++++++---------
>  tools/ccanlint/tests/module_builds.c |    2 +-
>  3 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/tools/ccanlint/ccanlint.h b/tools/ccanlint/ccanlint.h
> index ae22fa8..9cb858f 100644
> --- a/tools/ccanlint/ccanlint.h
> +++ b/tools/ccanlint/ccanlint.h
> @@ -152,6 +152,10 @@ char **per_file_options(const struct ccanlint *test, struct ccan_file *f);
>  void score_file_error(struct score *, struct ccan_file *f, unsigned line,
>  		      const char *errorfmt, ...);
>  
> +/* Append message to the score->error */
> +void score_error(struct score *score, const char * source,
> +		 const char *errorfmt, ...);
> +
>  /* Start a command in the background. */
>  void run_command_async(const void *ctx, unsigned int time_ms,
>  		       const char *fmt, ...);
> diff --git a/tools/ccanlint/file_analysis.c b/tools/ccanlint/file_analysis.c
> index 8fbb718..abcf079 100644
> --- a/tools/ccanlint/file_analysis.c
> +++ b/tools/ccanlint/file_analysis.c
> @@ -360,15 +360,9 @@ enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
>  	return ret;
>  }
>  
> -void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
> -		      const char *errorfmt, ...)
> +static void score_error_vfmt(struct score *score, const char *source,
> +			     const char *errorfmt, va_list ap)
>  {
> -	va_list ap;
> -
> -	struct file_error *fe = tal(score, struct file_error);
> -	fe->file = f;
> -	fe->line = line;
> -	list_add_tail(&score->per_file_errors, &fe->list);
>  
>  	if (!score->error)
>  		score->error = tal_strdup(score, "");
> @@ -384,17 +378,45 @@ void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
>  		return;
>  	}
>  
> +	tal_append_fmt(&score->error, "%s:", source);
> +	tal_append_vfmt(&score->error, errorfmt, ap);
> +	score->error = tal_strcat(score, take(score->error), "\n");
> +}
> +
> +
> +
> +void score_error(struct score *score, const char *source,
> +		 const char *errorfmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, errorfmt);
> +	score_error_vfmt(score, source, errorfmt, ap);
> +	va_end(ap);
> +}
> +
> +void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
> +		      const char *errorfmt, ...)
> +{
> +	va_list ap;
> +	char *source;
> +
> +	struct file_error *fe = tal(score, struct file_error);
> +	fe->file = f;
> +	fe->line = line;
> +	list_add_tail(&score->per_file_errors, &fe->list);
> +
>  	if (line)
> -		tal_append_fmt(&score->error, "%s:%u:", f->fullname, line);
> +		source = tal_fmt(score, "%s:%u", f->fullname, line);
>  	else
> -		tal_append_fmt(&score->error, "%s:", f->fullname);
> +		source = tal_fmt(score, "%s", f->fullname);
>  
>  	va_start(ap, errorfmt);
> -	tal_append_vfmt(&score->error, errorfmt, ap);
> +	score_error_vfmt(score, source, errorfmt, ap);
>  	va_end(ap);
> -	score->error = tal_strcat(score, take(score->error),"\n");
>  }
>  
> +
>  char *get_or_compile_info(const void *ctx, const char *dir)
>  {
>  	struct manifest *m = get_manifest(NULL, dir);
> diff --git a/tools/ccanlint/tests/module_builds.c b/tools/ccanlint/tests/module_builds.c
> index 45b67c5..cb13a0b 100644
> --- a/tools/ccanlint/tests/module_builds.c
> +++ b/tools/ccanlint/tests/module_builds.c
> @@ -57,7 +57,7 @@ static void do_build(struct manifest *m,
>  	m->compiled[COMPILE_NORMAL]
>  		= build_module(m, COMPILE_NORMAL, &errstr);
>  	if (!m->compiled[COMPILE_NORMAL]) {
> -		score_file_error(score, NULL, 0, "%s", errstr);
> +		score_error(score, m->modname,"%s", errstr);
>  		return;
>  	}
>  
> -- 
> 1.7.10.4
>
> _______________________________________________
> ccan mailing list
> ccan at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/ccan