On Mon, Jan 08, 2018 at 02:36:43PM +0100, Julia Lawall wrote: > Check for NULL position and file name. Check for xasprintf failure. > Builds on a patch proposed by Frank Rowand: > > https://www.mail-archive.com/devicetree-compiler@xxxxxxxxxxxxxxx/msg00377.html > > Annotation extension will introduce the possibility of the position > being NULL. > > Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx> > --- > srcpos.c | 35 +++++++++++++++++++++++------------ > 1 file changed, 23 insertions(+), 12 deletions(-) > > diff --git a/srcpos.c b/srcpos.c > index 9d38459..7f2626c 100644 > --- a/srcpos.c > +++ b/srcpos.c > @@ -249,24 +249,35 @@ srcpos_copy(struct srcpos *pos) > char * > srcpos_string(struct srcpos *pos) > { > - const char *fname = "<no-file>"; > + const char *fname; > char *pos_str; > - > - if (pos->file && pos->file->name) > + int rc; > + > + if (!pos) { > + rc = asprintf(&pos_str, "%s:<no-line>", fname); Uh.. you've removed the initializer of fname, so this branch will use it uninitialized, no? > + goto out; > + } else if (!pos->file) I prefer to keep braces on all branches if they're present on any branch. > + fname = "<no-file>"; > + else if (!pos->file->name) > + fname = "<no-filename>"; > + else > fname = pos->file->name; > > - > if (pos->first_line != pos->last_line) > - xasprintf(&pos_str, "%s:%d.%d-%d.%d", fname, > - pos->first_line, pos->first_column, > - pos->last_line, pos->last_column); > + rc = xasprintf(&pos_str, "%s:%d.%d-%d.%d", fname, > + pos->first_line, pos->first_column, > + pos->last_line, pos->last_column); > else if (pos->first_column != pos->last_column) > - xasprintf(&pos_str, "%s:%d.%d-%d", fname, > - pos->first_line, pos->first_column, > - pos->last_column); > + rc = xasprintf(&pos_str, "%s:%d.%d-%d", fname, > + pos->first_line, pos->first_column, > + pos->last_column); > else > - xasprintf(&pos_str, "%s:%d.%d", fname, > - pos->first_line, pos->first_column); > + rc = xasprintf(&pos_str, "%s:%d.%d", fname, > + pos->first_line, pos->first_column); > + > +out: > + if (rc == -1) > + die("Couldn't allocate in srcpos string"); You shouldn't need this - xasprintf() will already abort on allocation failure, that's the whole point of it. > return pos_str; > } -- 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:
signature.asc
Description: PGP signature
