Re: [PATCH v2 2/2] staging: gdm724x: gdm_tty: replaced macro with a function

[antoni.przyby...@wp.pl]

On 02.09.2020 18:21, Randy Dunlap wrote:

On 9/2/20 9:16 AM, Antoni Przybylik wrote:

Changed return type to bool and removed inline specifier. Also added
  static specifier.

why remove the inline specifier?


Greg KH wrote to me:

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


thanks.


Signed-off-by: Antoni Przybylik 
---
  drivers/staging/gdm724x/gdm_tty.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c 
b/drivers/staging/gdm724x/gdm_tty.c
index 34a13d98c029..179fc9b9400b 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -34,7 +34,7 @@ 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(struct gdm *gdm)

+static bool gdm_tty_ready(struct gdm *gdm)
  {
return (gdm && gdm->tty_dev && gdm->port.count);
  }




Antoni Przybylik
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Fwd: Re: [PATCH] staging: gdm724x: gdm_tty: corrected macro by adding brackets

[antoni.przyby...@wp.pl]

On 01.09.2020 13:08, Greg KH wrote:

On Tue, Sep 01, 2020 at 12:43:11PM +0200, antoniprzybylik wrote:

Such macros are dangerous. Consider following example:
#define GDM_TTY_READY(gdm) (gdm && gdm->tty_dev && gdm->port.count)
GDM_TTY_READY(a + b)
This macro will be expanded in such a way:
(a + b && a + b->tty_dev && a + b->port.count)
And it will lead to errors.

This is for a pointer, no one would ever do that :)


Nobody adds a pointer to a pointer, but it's common to add to it some 
value like that:


GDM_TTY_READY(myptr + 0x1000)


But, if you really worry about this, turn it into an inline function,
that way you get the proper typedef safety, which is what something like
this should really be, not a macro.


How to do it? Do I need to send another patch?

Antoni Przybylik

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm724x: gdm_tty: replaced macro with a function

[antoni.przyby...@wp.pl]

On 01.09.2020 19:50, Greg KH wrote:


On Tue, Sep 01, 2020 at 06:18:46PM +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 
---
  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..a7db0672e81d 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"};
  
+static int gdm_tty_ready(gdm *gdm)

+{
+   return (gdm && gdm->tty_dev && gdm->port.count);
+}

You obviously did not even build this patch, which is a bit rude, don't
you think?

:(


I'm stupid. I misconfigured the kernel. I fixed this bug and sent a new 
patch.

Linux configuration script is horrible...

Antoni Przybylik

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel