Re: [Qemu-devel] Re: [PATCH 02/11] qerror: expose a function to format an error

[Luiz Capitulino]

On Mon, 14 Mar 2011 14:27:30 -0500
Anthony Liguori <address@hidden> wrote:

> On 03/14/2011 02:17 PM, Luiz Capitulino wrote:
> > On Fri, 11 Mar 2011 15:08:38 -0600
> > Anthony Liguori<address@hidden>  wrote:
> >
> >> On 03/11/2011 03:00 PM, Anthony Liguori wrote:
> >>> This will let Error share the QError human formatting.  This is only used 
> >>> for
> >>> HMP.
> >>>
> >>> Signed-off-by: Anthony Liguori<address@hidden>
> >>>
> >>> diff --git a/qerror.c b/qerror.c
> >>> index 4855604..13d53c9 100644
> >>> --- a/qerror.c
> >>> +++ b/qerror.c
> >>> @@ -326,12 +326,18 @@ QError *qerror_from_info(const char *file, int 
> >>> linenr, const char *func,
> >>>        return qerr;
> >>>    }
> >>>
> >>> -static void parse_error(const QError *qerror, int c)
> >>> +static void parse_error(const QErrorStringTable *entry, int c)
> >>>    {
> >>> -    qerror_abort(qerror, "expected '%c' in '%s'", c, 
> >>> qerror->entry->desc);
> >>> +#if 0
> >>> +    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
> >>> +#else
> >>> +    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
> >>> +    abort();
> >>> +#endif
> >>>    }
> >> Err, I shouldn't have left these #if 0's in here.  Please ignore them.
> > But you're going to keep qerror_abort() usage, right?
> 
> No, qerror_abort() needs to go away.
> 
> It's too tied to QError and this patch is making the formatting code 
> work outside of of QEMU.

qerror_abort() only exists for debugging purposes. I won't say its perfect,
but it's better than nothing and has already saved some time when writing
new errors.

I'm fine dropping it as long as there's a better replacement, which is
not the case here. There's even a hunk that replaces qerror_abort() for
a plain abort().

> Once this whole series is completely merged, QError goes away entirely 
> and this pretty formatting is replaced with something much simpler.
> 
> Regards,
> 
> Anthony Liguori
>