[Libguestfs] [PATCH 1/3] builder/virt-index-validate: try to cleanup in any occasion
Pino Toscano
ptoscano at redhat.com
Thu Mar 20 15:24:10 UTC 2014
On Thursday 20 March 2014 14:15:29 Richard W.M. Jones wrote:
> On Thu, Mar 20, 2014 at 02:48:11PM +0100, Pino Toscano wrote:
> > Always close the file (ignoring its result) after a parsing, and
> > cleanup the parse_context object before any exit().
> >
> > This eases the debugging of memory issues in the actual parser.
> > ---
> >
> > builder/index-validate.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/builder/index-validate.c b/builder/index-validate.c
> > index 4b7fe93..fed0f81 100644
> > --- a/builder/index-validate.c
> > +++ b/builder/index-validate.c
> > @@ -62,6 +62,7 @@ main (int argc, char *argv[])
> >
> > struct section *sections;
> > struct parse_context context;
> > FILE *in;
> >
> > + int ret;
> >
> > setlocale (LC_ALL, "");
> > bindtextdomain (PACKAGE, LOCALEBASEDIR);
> >
> > @@ -109,19 +110,22 @@ main (int argc, char *argv[])
> >
> > exit (EXIT_FAILURE);
> >
> > }
> >
> > - if (do_parse (&context, in) != 0) {
> > - fprintf (stderr, _("%s: '%s' could not be validated, see errors
> > above\n"), + ret = do_parse (&context, in);
> > +
> > + if (fclose (in) == EOF) {
> > + fprintf (stderr, _("%s: %s: error closing input file: %m
> > (ignored)\n"),>
> > program_name, input);
> >
> > - exit (EXIT_FAILURE);
> >
> > }
> >
> > - if (fclose (in) == EOF) {
> > - fprintf (stderr, _("%s: %s: error reading input file: %m\n"),
> > + if (ret != 0) {
> > + parse_context_free (&context);
> > + fprintf (stderr, _("%s: '%s' could not be validated, see errors
> > above\n"),>
> > program_name, input);
> >
> > exit (EXIT_FAILURE);
> >
> > }
> >
> > if (compat_1_24_1 && context.seen_comments) {
> >
> > + parse_context_free (&context);
> >
> > fprintf (stderr, _("%s: %s contains comments which will not
> > work with virt-builder 1.24.1\n"),>
> > program_name, input);
> >
> > exit (EXIT_FAILURE);
> >
> > @@ -134,6 +138,7 @@ main (int argc, char *argv[])
> >
> > if (compat_1_24_0) {
> >
> > if (strchr (sections->name, '_')) {
> >
> > + parse_context_free (&context);
> >
> > fprintf (stderr, _("%s: %s: section [%s] has invalid
> > characters which will not work with virt-builder
> > 1.24.0\n"),>
> > program_name, input, sections->name);
> >
> > exit (EXIT_FAILURE);
> >
> > @@ -144,6 +149,7 @@ main (int argc, char *argv[])
> >
> > if (compat_1_24_0) {
> >
> > if (strchr (fields->key, '[') ||
> >
> > strchr (fields->key, ']')) {
> >
> > + parse_context_free (&context);
> >
> > fprintf (stderr, _("%s: %s: section [%s], field '%s' has
> > invalid characters which will not work with virt-builder
> > 1.24.0\n"),>
> > program_name, input, sections->name,
> > fields->key);
> >
> > exit (EXIT_FAILURE);
> >
> > @@ -152,6 +158,7 @@ main (int argc, char *argv[])
> >
> > if (compat_1_24_1) {
> >
> > if (strchr (fields->key, '.') ||
> >
> > strchr (fields->key, ',')) {
> >
> > + parse_context_free (&context);
> >
> > fprintf (stderr, _("%s: %s: section [%s], field '%s' has
> > invalid characters which will not work with virt-builder
> > 1.24.1\n"),>
> > program_name, input, sections->name,
> > fields->key);
> >
> > exit (EXIT_FAILURE);
> >
> > @@ -162,6 +169,7 @@ main (int argc, char *argv[])
> >
> > }
> >
> > if (compat_1_24_0 && !seen_sig) {
> >
> > + parse_context_free (&context);
> >
> > fprintf (stderr, _("%s: %s: section [%s] is missing a 'sig'
> > field which will not work with virt-builder 1.24.0\n"),>
> > program_name, input, sections->name);
> >
> > exit (EXIT_FAILURE);
>
> ACK.
>
> Two thoughts though:
>
> - Would a cleanup attribute be a better choice?
Possibly, although it didn't seem worth to me, for such a simple testing
tool.
> - Do we test this program under valgrind? If not, we probably should
> do.
Worth doing, patch coming shortly.
--
Pino Toscano
More information about the Libguestfs
mailing list