On Tue, 17 Sep 2019 13:29:36 +0200
Cornelia Huck <coh...@redhat.com> wrote:
> On Tue, 17 Sep 2019 12:22:18 +0200
> Greg Kurz <gr...@kaod.org> wrote:
>
> > Passing errp from the argument list to error_append_hint()
> > isn't recommended because it may cause unwanted behaviours
> > when errp is equal to &error_fatal or &error_abort.
> >
> > First, error_append_hint() aborts QEMU when passed either of
> > those.
> >
> > Second, consider the following:
> >
> > void foo(Error **errp)
> > {
> > error_setg(errp, "foo");
> > error_append_hint(errp, "Try bar\n");
> > }
> >
> > error_setg() causes QEMU to exit or abort, and hints aren't
> > added.
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> > scripts/checkpatch.pl | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index aa9a354a0e7e..17ce026282a6 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2902,6 +2902,10 @@ sub process {
> > }
> > }
> >
>
> Maybe add a comment here?
>
> # using errp is common practice, so that check should hopefully be enough
>
> > + if ($line =~ /error_append_hint\(errp/) {
> > + WARN("suspicious errp passed to error_append_hint()\n" .
>
> Add "(use a local error object)"?
>
Sure. I'll add both.
> > + $herecurr);
> > + }
> > # check for non-portable libc calls that have portable alternatives in QEMU
> > if ($line =~ /\bffs\(/) {
> > ERROR("use ctz32() instead of ffs()\n" . $herecurr);
> >
>
