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; } /**************************************************************************** _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel