| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5428E97F.7040608@suse.com>
Date: Mon, 29 Sep 2014 07:09:19 +0200
From: Juergen Gross <jgross@...e.com>
To: Chen Gang <gang.chen.5i5j@...il.com>,
David Vrabel <david.vrabel@...rix.com>, konrad.wilk@...cle.com,
ian.campbell@...rix.com, wei.liu2@...rix.com,
boris.ostrovsky@...cle.com, bhelgaas@...gle.com,
yongjun_wei@...ndmicro.com.cn, mukesh.rathor@...cle.com
CC: xen-devel@...ts.xenproject.org, linux-pci@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-scsi@...r.kernel.org,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH] xen/xenbus: Use 'void' instead of 'int' for
the return of xenbus_switch_state()
On 09/26/2014 06:36 PM, Chen Gang wrote:
> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
> internally, so need not return any status value, then use 'void' instead
> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
>
> Also need be sure that all callers which check the return value must let
> 'err' be 0.
>
> And also need change the related comments for xenbus_switch_state().
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@...il.com>
Acked-by: Juergen Gross <jgross@...e.com>
> ---
> drivers/block/xen-blkback/xenbus.c | 9 ++-------
> drivers/net/xen-netback/xenbus.c | 5 +----
> drivers/pci/xen-pcifront.c | 15 ++++++---------
> drivers/xen/xen-pciback/xenbus.c | 21 ++++-----------------
> drivers/xen/xen-scsiback.c | 5 +----
> drivers/xen/xenbus/xenbus_client.c | 16 ++++++++--------
> include/xen/xenbus.h | 3 ++-
> 7 files changed, 24 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 3a8b810..fdcc584 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -587,9 +587,7 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
> if (err)
> goto fail;
>
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto fail;
> + xenbus_switch_state(dev, XenbusStateInitWait);
>
> return 0;
>
> @@ -837,10 +835,7 @@ again:
> if (err)
> xenbus_dev_fatal(dev, err, "ending transaction");
>
> - err = xenbus_switch_state(dev, XenbusStateConnected);
> - if (err)
> - xenbus_dev_fatal(dev, err, "%s: switching to Connected state",
> - dev->nodename);
> + xenbus_switch_state(dev, XenbusStateConnected);
>
> return;
> abort:
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 9c47b89..b5c3d47 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
> if (err)
> pr_debug("Error writing multi-queue-max-queues\n");
>
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto fail;
> -
> + xenbus_switch_state(dev, XenbusStateInitWait);
> be->state = XenbusStateInitWait;
>
> /* This kicks hotplug scripts, so do it immediately. */
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 53df39a..c1d73b7 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -901,18 +901,16 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
> }
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> -
> + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> + return 0;
> out:
> return err;
> }
>
> static int pcifront_try_disconnect(struct pcifront_device *pdev)
> {
> - int err = 0;
> enum xenbus_state prev_state;
>
> -
> prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
>
> if (prev_state >= XenbusStateClosing)
> @@ -923,11 +921,10 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev)
> pcifront_disconnect(pdev);
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateClosed);
> + xenbus_switch_state(pdev->xdev, XenbusStateClosed);
>
> out:
> -
> - return err;
> + return 0;
> }
>
> static int pcifront_attach_devices(struct pcifront_device *pdev)
> @@ -1060,8 +1057,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
> domain, bus, slot, func);
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> -
> + xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> + return 0;
> out:
> return err;
> }
> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> index c214daa..630a215 100644
> --- a/drivers/xen/xen-pciback/xenbus.c
> +++ b/drivers/xen/xen-pciback/xenbus.c
> @@ -184,10 +184,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev)
>
> dev_dbg(&pdev->xdev->dev, "Connecting...\n");
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> - if (err)
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to connected state!");
> + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
>
> dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err);
> out:
> @@ -500,12 +497,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
> }
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
> - if (err) {
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to reconfigured state!");
> - goto out;
> - }
> + xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
>
> out:
> mutex_unlock(&pdev->dev_lock);
> @@ -640,10 +632,7 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev)
> goto out;
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
> - if (err)
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to initialised state!");
> + xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
>
> out:
> mutex_unlock(&pdev->dev_lock);
> @@ -683,9 +672,7 @@ static int xen_pcibk_xenbus_probe(struct xenbus_device *dev,
> }
>
> /* wait for xend to configure us */
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto out;
> + xenbus_switch_state(dev, XenbusStateInitWait);
>
> /* watch the backend node for backend configuration information */
> err = xenbus_watch_path(dev, dev->nodename, &pdev->be_watch,
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> index ad11258..847bc9c 100644
> --- a/drivers/xen/xen-scsiback.c
> +++ b/drivers/xen/xen-scsiback.c
> @@ -1225,10 +1225,7 @@ static int scsiback_probe(struct xenbus_device *dev,
> if (err)
> xenbus_dev_error(dev, err, "writing feature-sg-grant");
>
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto fail;
> -
> + xenbus_switch_state(dev, XenbusStateInitWait);
> return 0;
>
> fail:
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index ca74410..e2bcd1d 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(xenbus_watch_pathfmt);
> static void xenbus_switch_fatal(struct xenbus_device *, int, int,
> const char *, ...);
>
> -static int
> +static void
> __xenbus_switch_state(struct xenbus_device *dev,
> enum xenbus_state state, int depth)
> {
> @@ -188,7 +188,7 @@ __xenbus_switch_state(struct xenbus_device *dev,
> int err, abort;
>
> if (state == dev->state)
> - return 0;
> + return;
>
> again:
> abort = 1;
> @@ -196,7 +196,7 @@ again:
> err = xenbus_transaction_start(&xbt);
> if (err) {
> xenbus_switch_fatal(dev, depth, err, "starting transaction");
> - return 0;
> + return;
> }
>
> err = xenbus_scanf(xbt, dev->nodename, "state", "%d", ¤t_state);
> @@ -219,7 +219,7 @@ abort:
> } else
> dev->state = state;
>
> - return 0;
> + return;
> }
>
> /**
> @@ -228,12 +228,12 @@ abort:
> * @state: new state
> *
> * Advertise in the store a change of the given driver to the given new_state.
> - * Return 0 on success, or -errno on error. On error, the device will switch
> - * to XenbusStateClosing, and the error will be saved in the store.
> + * When failure occurs, it will call xenbus_switch_fatal() internally, so need
> + * not return value to notify upper caller.
> */
> -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
> +void xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
> {
> - return __xenbus_switch_state(dev, state, 0);
> + __xenbus_switch_state(dev, state, 0);
> }
>
> EXPORT_SYMBOL_GPL(xenbus_switch_state);
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index 0324c6d..587c53d 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -195,7 +195,8 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
> const char **, unsigned int),
> const char *pathfmt, ...);
>
> -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
> +void xenbus_switch_state(struct xenbus_device *dev,
> + enum xenbus_state new_state);
> int xenbus_grant_ring(struct xenbus_device *dev, unsigned long ring_mfn);
> int xenbus_map_ring_valloc(struct xenbus_device *dev,
> int gnt_ref, void **vaddr);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists