lkml.org 
[lkml]   [2022]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/3] serial: Add kserial_rs485 to avoid wasted space due to .padding
On Fri, Aug 26, 2022 at 5:51 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> Struct serial_rs485 has a .padding field to make uapi updates easier.

The struct

> It wastes space, however. Create struct kserial_rs485 which is a kerner
> counterpart w/o padding.
>
> kernel_serial_rs485_to_user_rs485()'s rs485 can now become const as
> padding is dealt within the local variable.

...

> -static int user_rs485_to_kernel_serial_rs485(struct serial_rs485 *rs485,
> +static int user_rs485_to_kernel_serial_rs485(struct kserial_rs485 *rs485,
> const struct serial_rs485 __user *rs485_user)
> {
> - if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
> + struct serial_rs485 rs485_uapi;
> +
> + if (copy_from_user(&rs485_uapi, rs485_user, sizeof(*rs485)))
> return -EFAULT;

> + *rs485 = *((struct kserial_rs485 *)&rs485_uapi);

So with all assets we have we can be sure that on BE64 / BE32 machines
this will be flawless. Is this assumption correct?

> return 0;
> }

...

> static int kernel_serial_rs485_to_user_rs485(struct serial_rs485 __user *rs485_user,
> - struct serial_rs485 *rs485)
> + const struct kserial_rs485 *rs485)
> {
> + struct serial_rs485 rs485_uapi;

> + *((struct kserial_rs485 *)&rs485_uapi) = *rs485;

Ditto.

+ Blank line?

> /* Return clean padding area to userspace */
> - memset(rs485->padding0, 0, sizeof(rs485->padding0));
> - memset(rs485->padding1, 0, sizeof(rs485->padding1));
> + memset(rs485_uapi.padding0, 0, sizeof(rs485_uapi.padding0));
> + memset(rs485_uapi.padding1, 0, sizeof(rs485_uapi.padding1));
>
> - if (copy_to_user(rs485_user, rs485, sizeof(*rs485)))
> + if (copy_to_user(rs485_user, &rs485_uapi, sizeof(rs485_uapi)))
> return -EFAULT;
>
> return 0;

...

> +/* Compile-time asserts for kserial_rs485 and serial_rs485 equality (except padding) */

struct kserial_rs485
struct serial_rs485

(rationale: standard representation in text / comments and be a link
in case if this is converted to kernel doc)

...

> +/*
> + * Must match with serial_rs485 in include/uapi/linux/serial.h excluding the

Ditto.

> + * padding.
> + */
> +struct kserial_rs485 {
> + __u32 flags; /* RS485 feature flags */
> + __u32 delay_rts_before_send; /* Delay before send (milliseconds) */
> + __u32 delay_rts_after_send; /* Delay after send (milliseconds) */
> + struct {
> + __u8 addr_recv;
> + __u8 addr_dest;
> + };

Btw, can't we convert them to kernel doc?

> +};

...

> + * There's kernel counterpart kserial_rs485 of this struct without padding.

struct kserial_rs485

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2022-08-26 17:51    [W:0.073 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site