Re: [PATCH 22/22] xlink-core: factorize xlink_ioctl function by creating sub-functions for each ioctl command

From: Joe Perches
Date: Sun Dec 06 2020 - 22:11:24 EST


On Tue, 2020-12-01 at 14:35 -0800, mgross@xxxxxxxxxxxxxxx wrote:
> From: Seamus Kelly <seamus.kelly@xxxxxxxxx>
>
> Refactor the too large IOCTL function to call helper functions.

This should not be sent as a known poor patch as patch 21 of 22
and then updated in patch 22 of 22 with a better style.

This should be sent in the as desired final form the first time
so that people don't give you useless notes.

> @@ -342,427 +323,84 @@ static int kmb_xlink_remove(struct platform_device *pdev)
>   * IOCTL function for User Space access to xlink kernel functions
>   *
>   */
> +int ioctl_connect(unsigned long arg);
>  
>
>  static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  {
> - struct xlink_handle devh = {0};
> - struct xlinkopenchannel op = {0};
> - struct xlinkwritedata wr = {0};
> - struct xlinkreaddata rd = {0};
> - struct xlinkreadtobuffer rdtobuf = {0};
> - struct xlinkconnect con = {0};
> - struct xlinkrelease rel = {0};
> - struct xlinkstartvpu startvpu = {0};
> - struct xlinkcallback cb = {0};
> - struct xlinkgetdevicename devn = {0};
> - struct xlinkgetdevicelist devl = {0};
> - struct xlinkgetdevicestatus devs = {0};
> - struct xlinkbootdevice boot = {0};
> - struct xlinkresetdevice res = {0};
> - struct xlinkdevmode devm = {0};
> - struct xlinkregdevevent regdevevent = {0};
> - u32 sw_device_id_list[XLINK_MAX_DEVICE_LIST_SIZE];
> - char name[XLINK_MAX_DEVICE_NAME_SIZE];
> - int interface = NULL_INTERFACE;
> - u32 device_status = 0;
> - u32 num_devices = 0;
> - u32 device_mode = 0;
> - u32 num_events = 0;
> - char filename[64];
> - u32 *ev_list;
> - u8 reladdr;
> - u8 *rdaddr;
> - u32 size;
>   int rc;
>  
>
>   switch (cmd) {
>   case XL_CONNECT:
> - if (copy_from_user(&con, (void __user *)arg,
> - sizeof(struct xlinkconnect)))
> - return -EFAULT;
> - if (copy_from_user(&devh, (void __user *)con.handle,
> - sizeof(struct xlink_handle)))
> - return -EFAULT;
> - rc = xlink_connect(&devh);
> - if (rc == X_LINK_SUCCESS) {
> - if (copy_to_user((void __user *)con.handle,
> - &devh, sizeof(struct xlink_handle)))
> - return -EFAULT;
> - }
> - if (copy_to_user((void __user *)con.return_code, (void *)&rc,
> - sizeof(rc)))
> - return -EFAULT;
> + rc = ioctl_connect(arg);
>   break;

etc...