[2] staging: vchiq_arm: cleanup vchiq_ioctl_create_service()

Dan Carpenter dan.carpenter at oracle.com
Mon Jan 23 03:42:00 PST 2017


In my last patch, I moved code to vchiq_ioctl_create_service() but
without cleaning it up or fixing any checkpatch warnings.  In this
patch I have cleaned the function up extensively.  There are no
functional changes:

1) We don't need the "ret" variable and can return directly.
2) Remove some stray curly braces.
3) Use sizeof(*user_service) instead of sizeof(USER_SERVICE_T).
4) Remove line breaks that aren't required and add tabs here and there.
5) I flipped the "if (service != NULL) {" condition around and return
   directly on error.  That lets us remove more line breakas.
6) I removed a "service = NULL;" assignment that isn't needed any more
   from before the return if vchiq_open_service_internal() fails.

It's several different kinds of cleanups but it's all local to one
function so I think it qualifies as "one thing per patch."

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index f9f69ca501b2..2cc43a724554 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -515,14 +515,11 @@ static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd, unsig
 	USER_SERVICE_T *user_service = NULL;
 	void *userdata;
 	int srvstate;
-	int ret = 0;
 
-	if (copy_from_user(&args, (const void __user *)arg,
-		  sizeof(args)) != 0) {
+	if (copy_from_user(&args, (const void __user *)arg, sizeof(args)))
 		return -EFAULT;
-	}
 
-	user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
+	user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
 	if (!user_service)
 		return -ENOMEM;
 
@@ -533,62 +530,52 @@ static int vchiq_ioctl_create_service(struct file *file, unsigned int cmd, unsig
 		}
 		srvstate = VCHIQ_SRVSTATE_OPENING;
 	} else {
-		srvstate =
-			 instance->connected ?
-			 VCHIQ_SRVSTATE_LISTENING :
-			 VCHIQ_SRVSTATE_HIDDEN;
+		srvstate = instance->connected ?
+				VCHIQ_SRVSTATE_LISTENING :
+				VCHIQ_SRVSTATE_HIDDEN;
 	}
 
 	userdata = args.params.userdata;
 	args.params.callback = service_callback;
 	args.params.userdata = user_service;
-	service = vchiq_add_service_internal(
-			instance->state,
-			&args.params, srvstate,
-			instance, user_service_free);
-
-	if (service != NULL) {
-		user_service->service = service;
-		user_service->userdata = userdata;
-		user_service->instance = instance;
-		user_service->is_vchi = (args.is_vchi != 0);
-		user_service->dequeue_pending = 0;
-		user_service->close_pending = 0;
-		user_service->message_available_pos =
-			instance->completion_remove - 1;
-		user_service->msg_insert = 0;
-		user_service->msg_remove = 0;
-		sema_init(&user_service->insert_event, 0);
-		sema_init(&user_service->remove_event, 0);
-		sema_init(&user_service->close_event, 0);
-
-		if (args.is_open) {
-			status = vchiq_open_service_internal
-				(service, instance->pid);
-			if (status != VCHIQ_SUCCESS) {
-				vchiq_remove_service(service->handle);
-				service = NULL;
-				ret = (status == VCHIQ_RETRY) ?
-					-EINTR : -EIO;
-				return ret;
-			}
-		}
+	service = vchiq_add_service_internal(instance->state,
+					     &args.params, srvstate,
+					     instance, user_service_free);
+	if (!service) {
+		kfree(user_service);
+		return -EEXIST;
+	}
 
-		if (copy_to_user((void __user *)
-			&(((VCHIQ_CREATE_SERVICE_T __user *)
-				arg)->handle),
-			(const void *)&service->handle,
-			sizeof(service->handle)) != 0) {
-			ret = -EFAULT;
+	user_service->service = service;
+	user_service->userdata = userdata;
+	user_service->instance = instance;
+	user_service->is_vchi = (args.is_vchi != 0);
+	user_service->dequeue_pending = 0;
+	user_service->close_pending = 0;
+	user_service->message_available_pos = instance->completion_remove - 1;
+	user_service->msg_insert = 0;
+	user_service->msg_remove = 0;
+	sema_init(&user_service->insert_event, 0);
+	sema_init(&user_service->remove_event, 0);
+	sema_init(&user_service->close_event, 0);
+	if (args.is_open) {
+		status = vchiq_open_service_internal(service, instance->pid);
+		if (status != VCHIQ_SUCCESS) {
 			vchiq_remove_service(service->handle);
+			if (status == VCHIQ_RETRY)
+				return -EINTR;
+			return  -EIO;
 		}
+	}
 
-	} else {
-		ret = -EEXIST;
-		kfree(user_service);
+	if (copy_to_user((void __user *)
+			 &(((VCHIQ_CREATE_SERVICE_T __user *)arg)->handle),
+			 &service->handle, sizeof(service->handle))) {
+		vchiq_remove_service(service->handle);
+		return -EFAULT;
 	}
 
-	return ret;
+	return 0;
 }
 
 /****************************************************************************



More information about the linux-rpi-kernel mailing list