qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	David Gibson <david@gibson.dropbear.id.au>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH 10/15] spapr: Add a return value to spapr_set_vcpu_id()
Date: Tue, 15 Sep 2020 15:53:52 +0200	[thread overview]
Message-ID: <20200915155352.605b0e47@bahia.lan> (raw)
In-Reply-To: <740605ab-5310-d2fe-eb20-138b8def0b48@redhat.com>

On Tue, 15 Sep 2020 15:08:05 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 9/14/20 2:35 PM, Greg Kurz wrote:
> > As recommended in "qapi/error.h", return true on success and false on
> > failure. This allows to reduce error propagation overhead in the callers.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  include/hw/ppc/spapr.h  | 2 +-
> >  hw/ppc/spapr.c          | 5 +++--
> >  hw/ppc/spapr_cpu_core.c | 5 +----
> >  3 files changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index c8cd63bc0667..11682f00e8cc 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -909,7 +909,7 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg);
> >  #define HTAB_SIZE(spapr)        (1ULL << ((spapr)->htab_shift))
> >  
> >  int spapr_get_vcpu_id(PowerPCCPU *cpu);
> > -void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp);
> > +bool spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp);
> 
> If you have to respin, please add some doc, at least this would
> be an improvement:
> 
> /* Returns: %true on success, %false on error. */
> 

Yeah, most, not to say all, APIs in the spapr code don't have
doc in the header files... which uselessly forces everyone to
check what the function actually does. Not sure how to best
address that though.

Adding headers everywhere (ie. lot of churn) ? Only in selected places
where it isn't obvious ? Also for functions that return integers or
pointers ?

I'll cowardly let David decide ;-)

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 



  reply	other threads:[~2020-09-15 13:54 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 12:34 [PATCH 00/15] spapr: Error handling fixes and cleanups (round 2) Greg Kurz
2020-09-14 12:34 ` [PATCH 01/15] spapr: Fix error leak in spapr_realize_vcpu() Greg Kurz
2020-09-15  9:08   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:00   ` Philippe Mathieu-Daudé
2020-09-14 12:34 ` [PATCH 02/15] ppc: Add a return value to ppc_set_compat() and ppc_set_compat_all() Greg Kurz
2020-09-15  9:18   ` Vladimir Sementsov-Ogievskiy
2020-09-15  9:34     ` Greg Kurz
2020-09-14 12:34 ` [PATCH 03/15] ppc: Fix return value in cpu_post_load() error path Greg Kurz
2020-09-15  9:50   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:01   ` Philippe Mathieu-Daudé
2020-09-14 12:34 ` [PATCH 04/15] spapr: Simplify error handling in callers of ppc_set_compat() Greg Kurz
2020-09-15  9:51   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:02   ` Philippe Mathieu-Daudé
2020-09-14 12:34 ` [PATCH 05/15] spapr: Get rid of cas_check_pvr() error reporting Greg Kurz
2020-09-15 10:03   ` Vladimir Sementsov-Ogievskiy
2020-09-15 11:00     ` [SPAM] " Greg Kurz
2020-09-14 12:34 ` [PATCH 06/15] spapr: Simplify error handling in do_client_architecture_support() Greg Kurz
2020-09-15 10:05   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:03   ` Philippe Mathieu-Daudé
2020-09-14 12:34 ` [PATCH 07/15] spapr: Simplify error handling in spapr_vio_busdev_realize() Greg Kurz
2020-09-15 10:15   ` Vladimir Sementsov-Ogievskiy
2020-09-14 12:34 ` [PATCH 08/15] spapr: Add a return value to spapr_drc_attach() Greg Kurz
2020-09-15 10:23   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:05   ` Philippe Mathieu-Daudé
2020-09-14 12:34 ` [PATCH 09/15] spapr: Simplify error handling in prop_get_fdt() Greg Kurz
2020-09-15 10:26   ` Vladimir Sementsov-Ogievskiy
2020-09-14 12:35 ` [PATCH 10/15] spapr: Add a return value to spapr_set_vcpu_id() Greg Kurz
2020-09-15 10:32   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:08   ` Philippe Mathieu-Daudé
2020-09-15 13:53     ` Greg Kurz [this message]
2020-09-17  1:06       ` David Gibson
2020-09-14 12:35 ` [PATCH 11/15] spapr: Simplify error handling in spapr_cpu_core_realize() Greg Kurz
2020-09-15 10:38   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:08   ` Philippe Mathieu-Daudé
2020-09-14 12:35 ` [PATCH 12/15] spapr: Add a return value to spapr_nvdimm_validate() Greg Kurz
2020-09-15 10:49   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:09   ` Philippe Mathieu-Daudé
2020-09-14 12:35 ` [PATCH 13/15] spapr: Add a return value to spapr_check_pagesize() Greg Kurz
2020-09-15 10:52   ` Vladimir Sementsov-Ogievskiy
2020-09-15 13:10   ` Philippe Mathieu-Daudé
2020-09-14 12:35 ` [PATCH 14/15] spapr: Simplify error handling in spapr_memory_plug() Greg Kurz
2020-09-15 10:58   ` Vladimir Sementsov-Ogievskiy
2020-09-15 11:43     ` [SPAM] " Greg Kurz
2020-09-15 11:53       ` Vladimir Sementsov-Ogievskiy
2020-09-15 12:04         ` Greg Kurz
2020-09-15 13:43           ` Greg Kurz
2020-09-17  1:15       ` [SPAM] " David Gibson
2020-09-17  7:38         ` Markus Armbruster
2020-09-17 10:04           ` Greg Kurz
2020-09-17 12:04             ` Markus Armbruster
2020-09-17 12:18               ` Daniel P. Berrangé
2020-09-17 12:40                 ` Greg Kurz
2020-09-17 13:17                 ` Markus Armbruster
2020-09-14 12:35 ` [PATCH 15/15] spapr: Simplify error handling in spapr_memory_unplug_request() Greg Kurz
2020-09-16  2:49 ` [PATCH 00/15] spapr: Error handling fixes and cleanups (round 2) David Gibson
2020-09-17  1:08   ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200915155352.605b0e47@bahia.lan \
    --to=groug@kaod.org \
    --cc=armbru@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).