2020-09-01 15:51:34

by Antoni Przybylik

[permalink] [raw]
Subject: [PATCH] staging: gdm724x: gdm_tty: replaced macro with an inline function

This approach is more elegant and prevents some problems related to
macros such as operator precedence in expanded expression.

Signed-off-by: Antoni Przybylik <[email protected]>
---
drivers/staging/gdm724x/gdm_tty.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 6e813693a766..e789b7348dee 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -27,8 +27,6 @@

#define MUX_TX_MAX_SIZE 2048

-#define GDM_TTY_READY(gdm) (gdm && gdm->tty_dev && gdm->port.count)
-
static struct tty_driver *gdm_driver[TTY_MAX_COUNT];
static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
static DEFINE_MUTEX(gdm_table_lock);
@@ -36,6 +34,11 @@ static DEFINE_MUTEX(gdm_table_lock);
static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};

+inline int gdm_tty_ready(gdm *gdm)
+{
+ return (gdm && gdm->tty_dev && gdm->port.count);
+}
+
static void gdm_port_destruct(struct tty_port *port)
{
struct gdm *gdm = container_of(port, struct gdm, port);
@@ -119,7 +122,7 @@ static int gdm_tty_recv_complete(void *data,
{
struct gdm *gdm = tty_dev->gdm[index];

- if (!GDM_TTY_READY(gdm)) {
+ if (!gdm_tty_ready(gdm)) {
if (complete == RECV_PACKET_PROCESS_COMPLETE)
gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev,
gdm_tty_recv_complete);
@@ -146,7 +149,7 @@ static void gdm_tty_send_complete(void *arg)
{
struct gdm *gdm = arg;

- if (!GDM_TTY_READY(gdm))
+ if (!gdm_tty_ready(gdm))
return;

tty_port_tty_wakeup(&gdm->port);
@@ -160,7 +163,7 @@ static int gdm_tty_write(struct tty_struct *tty, const unsigned char *buf,
int sent_len = 0;
int sending_len = 0;

- if (!GDM_TTY_READY(gdm))
+ if (!gdm_tty_ready(gdm))
return -ENODEV;

if (!len)
@@ -187,7 +190,7 @@ static int gdm_tty_write_room(struct tty_struct *tty)
{
struct gdm *gdm = tty->driver_data;

- if (!GDM_TTY_READY(gdm))
+ if (!gdm_tty_ready(gdm))
return -ENODEV;

return WRITE_SIZE;
--
2.28.0


2020-09-01 16:05:46

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: gdm724x: gdm_tty: replaced macro with an inline function

On Tue, Sep 01, 2020 at 05:44:37PM +0200, Antoni Przybylik wrote:
> This approach is more elegant and prevents some problems related to
> macros such as operator precedence in expanded expression.
>
> Signed-off-by: Antoni Przybylik <[email protected]>
> ---
> drivers/staging/gdm724x/gdm_tty.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> index 6e813693a766..e789b7348dee 100644
> --- a/drivers/staging/gdm724x/gdm_tty.c
> +++ b/drivers/staging/gdm724x/gdm_tty.c
> @@ -27,8 +27,6 @@
>
> #define MUX_TX_MAX_SIZE 2048
>
> -#define GDM_TTY_READY(gdm) (gdm && gdm->tty_dev && gdm->port.count)
> -
> static struct tty_driver *gdm_driver[TTY_MAX_COUNT];
> static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
> static DEFINE_MUTEX(gdm_table_lock);
> @@ -36,6 +34,11 @@ static DEFINE_MUTEX(gdm_table_lock);
> static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
> static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};
>
> +inline int gdm_tty_ready(gdm *gdm)

Why is this a global function?

And really, no need to make it inline, just make it a normal function
and the compiler will inline it if needed.

thanks,

greg k-h

2020-09-01 19:27:46

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] staging: gdm724x: gdm_tty: replaced macro with an inline function

Hi Antoni,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url: https://github.com/0day-ci/linux/commits/Antoni-Przybylik/staging-gdm724x-gdm_tty-replaced-macro-with-an-inline-function/20200901-234632
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git cc34073c6248e9cec801bf690d1455f264d12357
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/staging/gdm724x/gdm_tty.c:37:26: error: unknown type name 'gdm'
37 | inline int gdm_tty_ready(gdm *gdm)
| ^~~
drivers/staging/gdm724x/gdm_tty.c: In function 'gdm_tty_recv_complete':
>> drivers/staging/gdm724x/gdm_tty.c:125:7: error: implicit declaration of function 'gdm_tty_ready' [-Werror=implicit-function-declaration]
125 | if (!gdm_tty_ready(gdm)) {
| ^~~~~~~~~~~~~
cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/b5088fce047a55ac2021d1b82d39a39d2a5dbf4d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Antoni-Przybylik/staging-gdm724x-gdm_tty-replaced-macro-with-an-inline-function/20200901-234632
git checkout b5088fce047a55ac2021d1b82d39a39d2a5dbf4d
vim +/gdm +37 drivers/staging/gdm724x/gdm_tty.c

36
> 37 inline int gdm_tty_ready(gdm *gdm)
38 {
39 return (gdm && gdm->tty_dev && gdm->port.count);
40 }
41
42 static void gdm_port_destruct(struct tty_port *port)
43 {
44 struct gdm *gdm = container_of(port, struct gdm, port);
45
46 mutex_lock(&gdm_table_lock);
47 gdm_table[gdm->index][gdm->minor] = NULL;
48 mutex_unlock(&gdm_table_lock);
49
50 kfree(gdm);
51 }
52
53 static const struct tty_port_operations gdm_port_ops = {
54 .destruct = gdm_port_destruct,
55 };
56
57 static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
58 {
59 struct gdm *gdm = NULL;
60 int ret;
61
62 ret = match_string(DRIVER_STRING, TTY_MAX_COUNT,
63 tty->driver->driver_name);
64 if (ret < 0)
65 return -ENODEV;
66
67 mutex_lock(&gdm_table_lock);
68 gdm = gdm_table[ret][tty->index];
69 if (!gdm) {
70 mutex_unlock(&gdm_table_lock);
71 return -ENODEV;
72 }
73
74 tty_port_get(&gdm->port);
75
76 ret = tty_standard_install(driver, tty);
77 if (ret) {
78 tty_port_put(&gdm->port);
79 mutex_unlock(&gdm_table_lock);
80 return ret;
81 }
82
83 tty->driver_data = gdm;
84 mutex_unlock(&gdm_table_lock);
85
86 return 0;
87 }
88
89 static int gdm_tty_open(struct tty_struct *tty, struct file *filp)
90 {
91 struct gdm *gdm = tty->driver_data;
92
93 return tty_port_open(&gdm->port, tty, filp);
94 }
95
96 static void gdm_tty_cleanup(struct tty_struct *tty)
97 {
98 struct gdm *gdm = tty->driver_data;
99
100 tty_port_put(&gdm->port);
101 }
102
103 static void gdm_tty_hangup(struct tty_struct *tty)
104 {
105 struct gdm *gdm = tty->driver_data;
106
107 tty_port_hangup(&gdm->port);
108 }
109
110 static void gdm_tty_close(struct tty_struct *tty, struct file *filp)
111 {
112 struct gdm *gdm = tty->driver_data;
113
114 tty_port_close(&gdm->port, tty, filp);
115 }
116
117 static int gdm_tty_recv_complete(void *data,
118 int len,
119 int index,
120 struct tty_dev *tty_dev,
121 int complete)
122 {
123 struct gdm *gdm = tty_dev->gdm[index];
124
> 125 if (!gdm_tty_ready(gdm)) {
126 if (complete == RECV_PACKET_PROCESS_COMPLETE)
127 gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev,
128 gdm_tty_recv_complete);
129 return TO_HOST_PORT_CLOSE;
130 }
131
132 if (data && len) {
133 if (tty_buffer_request_room(&gdm->port, len) == len) {
134 tty_insert_flip_string(&gdm->port, data, len);
135 tty_flip_buffer_push(&gdm->port);
136 } else {
137 return TO_HOST_BUFFER_REQUEST_FAIL;
138 }
139 }
140
141 if (complete == RECV_PACKET_PROCESS_COMPLETE)
142 gdm->tty_dev->recv_func(gdm->tty_dev->priv_dev,
143 gdm_tty_recv_complete);
144
145 return 0;
146 }
147

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.01 kB)
.config.gz (63.81 kB)
Download all attachments