[PATCH] staging: fbtft: fix coding style issue in fbtft-bus.c

[PATCH] staging: fbtft: fix coding style issue in fbtft-bus.c

[Baker]

Remove trailing space and comma before closing parenthesis ')' in
define_fbtft_write_reg macro as reported by checkpatch.pl.

Signed-off-by: Baker <mzndmzn@gmail.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 30e436ff19e4..409770891c54 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -62,9 +62,9 @@ out:									      \
 }                                                                             \
 EXPORT_SYMBOL(func);
 
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8)
 define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16)
-define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, )
+define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16)
 
 void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
 {
-- 
2.47.3

[PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Baker]

The define_fbtft_write_reg macro calls 'modifier' as a function.
Passing an empty token as modifier is undefined behavior in C for
fixed-arity macros. Introduce fbtft_no_conv() as an identity
function to replace the empty args in the no-conversion cases.

Signed-off-by: Baker <mzndmzn@gmail.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 30e436ff19e4..7f3fd30576ab 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -11,6 +11,8 @@
  *
  *****************************************************************************/
 
+#define fbtft_no_conv(x) (x)
+
 #define define_fbtft_write_reg(func, buffer_type, data_type, modifier)        \
 void func(struct fbtft_par *par, int len, ...)                                \
 {                                                                             \
@@ -62,9 +64,9 @@ out:									      \
 }                                                                             \
 EXPORT_SYMBOL(func);
 
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, fbtft_no_conv)
 define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16)
-define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, )
+define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, fbtft_no_conv)
 
 void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
 {
-- 
2.47.3

Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Andy Shevchenko]

On Sun, Apr 12, 2026 at 8:33 PM Baker <mzndmzn@gmail.com> wrote:
>
> The define_fbtft_write_reg macro calls 'modifier' as a function.
> Passing an empty token as modifier is undefined behavior in C for
> fixed-arity macros. Introduce fbtft_no_conv() as an identity
> function to replace the empty args in the no-conversion cases.

This trick is used in the kernel (usually for u8). If you really want
to improve, use _Generic() instead.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Greg KH]

On Mon, Apr 13, 2026 at 08:58:50AM +0300, Andy Shevchenko wrote:
> On Sun, Apr 12, 2026 at 8:33 PM Baker <mzndmzn@gmail.com> wrote:
> >
> > The define_fbtft_write_reg macro calls 'modifier' as a function.
> > Passing an empty token as modifier is undefined behavior in C for
> > fixed-arity macros. Introduce fbtft_no_conv() as an identity
> > function to replace the empty args in the no-conversion cases.
> 
> This trick is used in the kernel (usually for u8). If you really want
> to improve, use _Generic() instead.

No, sorry, this code is fine as-is.

greg k-h

Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Andy Shevchenko]

On Mon, Apr 13, 2026 at 9:03 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Apr 13, 2026 at 08:58:50AM +0300, Andy Shevchenko wrote:
> > On Sun, Apr 12, 2026 at 8:33 PM Baker <mzndmzn@gmail.com> wrote:
> > >
> > > The define_fbtft_write_reg macro calls 'modifier' as a function.
> > > Passing an empty token as modifier is undefined behavior in C for
> > > fixed-arity macros. Introduce fbtft_no_conv() as an identity
> > > function to replace the empty args in the no-conversion cases.
> >
> > This trick is used in the kernel (usually for u8). If you really want
> > to improve, use _Generic() instead.
>
> No, sorry, this code is fine as-is.

Works for me. I suggested they explore the possibilities, it might be
helpful somewhere else in case of the similar requirements (like in a
new code).

-- 
With Best Regards,
Andy Shevchenko

Claude review: Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Claude Code Review Bot]

Overall Series Review

Subject: Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter
Author: Andy Shevchenko <andy.shevchenko@gmail.com>
Patches: 5
Reviewed: 2026-04-13T18:50:47.485308

---

This mbox contains two patches addressing the same issue in `drivers/staging/fbtft/fbtft-bus.c`: the `define_fbtft_write_reg` macro is invoked with an empty 4th argument for the no-conversion cases. Patch 1 is a broken v1 that removes the argument entirely, while Patch 2 (labeled v3) is the corrected approach that introduces an identity macro. Only Patch 2 should be considered for merging.

The series has formatting/process issues: Patch 1 is a standalone `[PATCH]` (not a cover letter) that contains a full diff of a broken approach, and Patch 2 is `[PATCH v3]` sent as a reply. This is confusing — v3 should have been sent as its own top-level thread, with a note that it supersedes the prior versions.

Patch 2 is functionally correct but its commit message makes an inaccurate claim about undefined behavior.

---
Generated by Claude Code Patch Reviewer

Claude review: Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Claude Code Review Bot]

Patch Review

**Verdict: NAK — this patch is broken and must not be applied.**

The macro `define_fbtft_write_reg` is defined with 4 parameters:

```c
#define define_fbtft_write_reg(func, buffer_type, data_type, modifier)
```

And `modifier` is used as a function-like call in the body (line 25, 42, 55 of the source):

```c
buf[i] = modifier((data_type)va_arg(args, unsigned int));
```

The patch changes:
```c
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8)
```

This passes **3 arguments** to a **4-parameter macro**, which is a preprocessor error in standard C (C99 §6.10.3). This would fail to compile. The trailing comma with an empty token was not just whitespace — it was the empty 4th argument, which expands to nothing and effectively makes `modifier(expr)` become just `(expr)`.

The commit message describes this as fixing a "coding style issue reported by checkpatch.pl," but the proposed fix introduces a build break.

---

---
Generated by Claude Code Patch Reviewer

Claude review: Re: [PATCH v3] staging: fbtft: replace empty macro args with identity converter

[Claude Code Review Bot]

Patch Review

**Verdict: The approach is correct but the commit message needs revision.**

The change itself is sound: introducing `#define fbtft_no_conv(x) (x)` as an identity macro and using it in place of the empty argument:

```c
+#define fbtft_no_conv(x) (x)
...
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, fbtft_no_conv)
...
-define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, )
+define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, fbtft_no_conv)
```

The expansion is functionally identical: `fbtft_no_conv((u8)va_arg(args, unsigned int))` expands to `((u8)va_arg(args, unsigned int))`, which is the same result as the empty-argument expansion.

**Issues:**

1. **Inaccurate commit message**: The commit message states *"Passing an empty token as modifier is undefined behavior in C for fixed-arity macros."* This is incorrect. C99 (§6.10.3) explicitly permits empty macro arguments — the argument count still matches (4 args to 4 params; the 4th is just an empty token sequence). The existing code is technically valid C99. The change is still worthwhile for **clarity and readability** — making the intent explicit with a named identity conversion is better than relying on the subtle empty-argument expansion trick — but the justification should be corrected. Claiming UB when there is none weakens the argument for the patch.

2. **Minor style nit**: The `fbtft_no_conv` macro is placed above the `define_fbtft_write_reg` macro definition (after the file header comment block), which is fine placement-wise. However, since `cpu_to_be16` is a kernel-provided macro/function, it might be worth adding a brief comment like `/* identity conversion for cases that need no byte-order change */` to explain why this exists, especially for a staging driver where cleanup patches are common and future readers may wonder what it's for.

3. **Process**: The `[PATCH v3]` label implies there were v1 and v2 before, but no changelog below the `---` line explains what changed between versions. For staging patches this is less critical, but it's good practice.

---
Generated by Claude Code Patch Reviewer