linux-kernel - [PATCH RFC V2] staging: kpc2000: use int for wait_for_completion

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1556356474-8575-1-git-send-email-hofrat@osadl.org>
Date:   Sat, 27 Apr 2019 11:14:34 +0200
From:   Nicholas Mc Guire <hofrat@...dl.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Matt Sickler <Matt.Sickler@...tronics.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH RFC V2] staging: kpc2000: use int for wait_for_completion_interruptible

weit_for_completion_interruptible returns in (0 on completion and 
-ERESTARTSYS on interruption) - so use an int not long for API conformance
and simplify the logic here a bit: need not check explicitly for == 0 as
this is either -ERESTARTSYS or 0.

Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---

Problem located with experimental API conformance checking cocci script

V2: kbuild reported a missing closing comment - seems that I managed
    send out the the initial version before compile testing/checkpatching
    sorry for the noise.

Not sure if making such point-wise fixes makes much sense - this driver has
a number of issues both style-wise and API compliance wise.

Note that kpc_dma_transfer() returns int not long - currently rv (long) is
being returned in most places (some places do return int) - so the return
handling here is a bit inconsistent.

Patch was compile-tested with: x86_64_defconfig + KPC2000=y, KPC2000_DMA=y
(with a number of unrelated sparse warnings about non-declared symbols, and
 smatch warnings about overflowing constants as well as coccicheck warning
 about usless casting)

Patch is against 5.1-rc6 (localversion-next is next-20190426)

 drivers/staging/kpc2000/kpc_dma/fileops.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 5741d2b..66f0d5a 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -38,6 +38,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct kiocb *kcb, unsigned
 {
 	unsigned int i = 0;
 	long rv = 0;
+	int ret = 0;
 	struct kpc_dma_device *ldev;
 	struct aio_cb_data *acd;
 	DECLARE_COMPLETION_ONSTACK(done);
@@ -180,16 +181,17 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct kiocb *kcb, unsigned
 	
 	// If this is a synchronous kiocb, we need to put the calling process to sleep until the transfer is complete
 	if (kcb == NULL || is_sync_kiocb(kcb)){
-		rv = wait_for_completion_interruptible(&done);
-		// If the user aborted (rv == -ERESTARTSYS), we're no longer responsible for cleaning up the acd
-		if (rv == -ERESTARTSYS){
+		ret = wait_for_completion_interruptible(&done);
+		/* If the user aborted (ret == -ERESTARTSYS), we're
+		 * no longer responsible for cleaning up the acd
+		 */
+		if (ret) {
 			acd->cpl = NULL;
-		}
-		if (rv == 0){
-			rv = acd->len;
+		} else {
+			ret = acd->len;
 			kfree(acd);
 		}
-		return rv;
+		return ret;
 	}
 	
 	return -EIOCBQUEUED;
-- 
2.1.4