linux-kernel - Re: [PATCH v1 4/4] serial: exar: change port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 19 Apr 2024 08:07:50 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Parker Newman <parker@...est.io>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Cc: Parker Newman <pnewman@...necttech.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v1 4/4] serial: exar: change port_type ternary line
 wrapping

On 18. 04. 24, 17:36, Parker Newman wrote:
> From: Parker Newman <pnewman@...necttech.com>
> 
> Change line wrapping of ternary operators in
> cti_get_port_type_xr17c15x_xr17v25x() for better readability.
> 
> Old example:
> 
> 	port_type = port_num == 0 ?
> 			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> 
> New:
> 	port_type = port_num == 0 ? CTI_PORT_TYPE_RS232 :
> 				CTI_PORT_TYPE_RS422_485;

This is worse IMO. Ilpo suggested a bit different alignment. But still...

> Based on feedback from:
> Link: https://lore.kernel.org/linux-serial/f2353b8c-2079-b895-2707-f6be83161288@linux.intel.com

You should have CCed the author.

> Signed-off-by: Parker Newman <pnewman@...necttech.com>
> ---
>   drivers/tty/serial/8250/8250_exar.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 10725ad0f3ef..a76b4e5bab4e 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -741,19 +741,19 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
>   		break;
>   	// 1x RS232, 1x RS422/RS485
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_1_1:
> -		port_type = port_num == 0 ?
> -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> +		port_type = port_num == 0 ? CTI_PORT_TYPE_RS232 :
> +					CTI_PORT_TYPE_RS422_485;


Well, could you initialize port_type = CTI_PORT_TYPE_RS232? And here do 
only:
if (port_num > 0)
     return CTI_PORT_TYPE_RS422_485;
?


>   		break;
>   	// 2x RS232, 2x RS422/RS485
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_2:
> -		port_type = port_num < 2 ?
> -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> +		port_type = port_num < 2 ? CTI_PORT_TYPE_RS232 :
> +					CTI_PORT_TYPE_RS422_485;

And so on.

>   		break;
>   	// 4x RS232, 4x RS422/RS485
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4:
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP:
> -		port_type = port_num < 4 ?
> -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> +		port_type = port_num < 4 ? CTI_PORT_TYPE_RS232 :
> +					CTI_PORT_TYPE_RS422_485;
>   		break;
>   	// RS232/RS422/RS485 HW (jumper) selectable
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2:
> @@ -789,13 +789,13 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
>   		break;
>   	// 6x RS232, 2x RS422/RS485
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP:
> -		port_type = port_num < 6 ?
> -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> +		port_type = port_num < 6 ? CTI_PORT_TYPE_RS232 :
> +					CTI_PORT_TYPE_RS422_485;
>   		break;
>   	// 2x RS232, 6x RS422/RS485
>   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP:
> -		port_type = port_num < 2 ?
> -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> +		port_type = port_num < 2 ? CTI_PORT_TYPE_RS232 :
> +					CTI_PORT_TYPE_RS422_485;
>   		break;
>   	default:
>   		dev_err(&pcidev->dev, "unknown/unsupported device\n");
> --
> 2.43.2
> 

-- 
js
suse labs