[PATCH] serial: 8250: Optimize port function assignment with generic macro

[PATCH] serial: 8250: Optimize port function assignment with generic macro

[xiaopeitux]

From: xiaopei01 <xiaopei01@kylinos.cn>

Refactor repetitive conditional function pointer assignments using a
generic macro ASSIGN_IF_EXIST. This consolidates 15+ conditional
checks into a consistent pattern while maintaining type safety.

Signed-off-by: xiaopei01 <xiaopei01@kylinos.cn>
---
 drivers/tty/serial/8250/8250_core.c | 52 ++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 6f676bb..6f305e9 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -777,38 +777,30 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 
 		serial8250_set_defaults(uart);
 
+		#define ASSIGN_IF_EXIST(dest, src, member) \
+			do { \
+				if ((src)->member) \
+					(dest)->member = (src)->member; \
+			} while (0)
+
 		/* Possibly override default I/O functions.  */
-		if (up->port.serial_in)
-			uart->port.serial_in = up->port.serial_in;
-		if (up->port.serial_out)
-			uart->port.serial_out = up->port.serial_out;
-		if (up->port.handle_irq)
-			uart->port.handle_irq = up->port.handle_irq;
+		ASSIGN_IF_EXIST(&uart->port, &up->port, serial_in);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, serial_out);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, handle_irq);
 		/*  Possibly override set_termios call */
-		if (up->port.set_termios)
-			uart->port.set_termios = up->port.set_termios;
-		if (up->port.set_ldisc)
-			uart->port.set_ldisc = up->port.set_ldisc;
-		if (up->port.get_mctrl)
-			uart->port.get_mctrl = up->port.get_mctrl;
-		if (up->port.set_mctrl)
-			uart->port.set_mctrl = up->port.set_mctrl;
-		if (up->port.get_divisor)
-			uart->port.get_divisor = up->port.get_divisor;
-		if (up->port.set_divisor)
-			uart->port.set_divisor = up->port.set_divisor;
-		if (up->port.startup)
-			uart->port.startup = up->port.startup;
-		if (up->port.shutdown)
-			uart->port.shutdown = up->port.shutdown;
-		if (up->port.pm)
-			uart->port.pm = up->port.pm;
-		if (up->port.handle_break)
-			uart->port.handle_break = up->port.handle_break;
-		if (up->dl_read)
-			uart->dl_read = up->dl_read;
-		if (up->dl_write)
-			uart->dl_write = up->dl_write;
+		ASSIGN_IF_EXIST(&uart->port, &up->port, set_termios);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, set_ldisc);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, get_mctrl);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, set_mctrl);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, get_divisor);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, set_divisor);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, startup);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, shutdown);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, pm);
+		ASSIGN_IF_EXIST(&uart->port, &up->port, handle_break);
+
+		ASSIGN_IF_EXIST(uart, up, dl_read);
+		ASSIGN_IF_EXIST(uart, up, dl_write);
 
 		if (uart->port.type != PORT_8250_CIR) {
 			if (uart_console_registered(&uart->port))
-- 
2.7.4

Re: [PATCH] serial: 8250: Optimize port function assignment with generic macro

[Greg KH]

On Thu, Mar 06, 2025 at 11:00:32AM +0800, xiaopeitux@foxmail.com wrote:
> From: xiaopei01 <xiaopei01@kylinos.cn>

Sorry, as per the documentation we need a name, not just an email alias.

> Refactor repetitive conditional function pointer assignments using a
> generic macro ASSIGN_IF_EXIST. This consolidates 15+ conditional
> checks into a consistent pattern while maintaining type safety.

But why?  Macros are a pain and hide what is happening here.  Do you
think this makes the code more maintainable over time?  Does it fix any
existing bugs?

thanks,

greg k-h

Re: [PATCH] serial: 8250: Optimize port function assignment with generic macro

[Pei Xiao]

在 2025/3/6 15:05, Greg KH 写道:
> On Thu, Mar 06, 2025 at 11:00:32AM +0800, xiaopeitux@foxmail.com wrote:
>> From: xiaopei01 <xiaopei01@kylinos.cn>
> Sorry, as per the documentation we need a name, not just an email alias.

ok，perhaps I changed a new computer to send patch caused it. 

I will modify in next time!

>> Refactor repetitive conditional function pointer assignments using a
>> generic macro ASSIGN_IF_EXIST. This consolidates 15+ conditional
>> checks into a consistent pattern while maintaining type safety.
> But why?  Macros are a pain and hide what is happening here.  Do you
> think this makes the code more maintainable over time?  Does it fix any
> existing bugs?

sorry for make some noise，I know you're a very busy person. 

Sorry for disturbing you！

No,it don't fix any bugs.Indeed, it's less readable than the previous code. 

Initially, I thought there were too many repetitive lines, 

and the code length could be reduced.


thanks！

Pei Xiao

> thanks,
>
> greg k-h