Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

From: Thomas Gleixner
Date: Tue Sep 19 2023 - 15:51:19 EST


On Tue, Sep 19 2023 at 16:24, Petr Mladek wrote:
> On Thu 2023-09-14 20:43:18, John Ogness wrote:
> IMHO, it would have been better to pass the flags variable directly
> via a macro as it is done in most *_lock_*_irqsafe() APIs. I mean
> something like:
>
> /**
> * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> * @up: Pointer to UART port structure
> * @flags: Interrupt flags storage
> *
> * Returns: True if lock was acquired, false otherwise
> */
> #define uart_port_lock_irqsave(up, flags) \
> ({ \
> local_irq_save(flags); \
> uart_port_lock(lock) \
> })

It's worse.

1) Macros are not type safe by themself and rely on type safety
of the inner workings.

2) Macros are bad for grep as you can't search for a 'struct foo *'
argument. Even semantic parsers have their problems with macro
constructs. I just learned that again when doing this.

3) Macros are just horrible to read

4) If you want to out of line the wrapper later, then you still
have to keep the macro around because the 'flags' argument is by
value and not a pointer.

>From a code generation point of view it's completely irrelevant whether
you have a macro or an inline. That was different 25 years ago, but who
cares about museum compilers today.

Thanks,

tglx