DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
@ 2019-11-04 16:33 Jules Irenge
  2019-11-04 16:46 ` Greg KH
  2019-11-04 17:03 ` Ian Abbott
  0 siblings, 2 replies; 6+ messages in thread
From: Jules Irenge @ 2019-11-04 16:33 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: devel, Jules Irenge, gregkh, linux-kernel, abbotti

Rewrite macro function with the GNU extension typeof
to remove a possible side-effects of MACRO argument reuse "x".
 - Problem could rise if arguments have different types
and different use though.

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
v1 - had no full commit log message, with changes not intended to be in the patch 
v2 - remove some changes not intended to be in this driver
     include note of a potential problem
 drivers/staging/comedi/comedi.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index 09a940066c0e..a57691a2e8d8 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -1103,8 +1103,10 @@ enum ni_common_signal_names {
 
 /* *** END GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
 
-#define NI_USUAL_PFI_SELECT(x)	(((x) < 10) ? (0x1 + (x)) : (0xb + (x)))
-#define NI_USUAL_RTSI_SELECT(x)	(((x) < 7) ? (0xb + (x)) : 0x1b)
+#define NI_USUAL_PFI_SELECT(x)\
+	({typeof(x) x_ = (x); (x_ < 10) ? (0x1 + x_) : (0xb + x_); })
+#define NI_USUAL_RTSI_SELECT(x)\
+	({typeof(x) x_ = (x); (x_ < 7) ? (0xb + x_) : 0x1b; })
 
 /*
  * mode bits for NI general-purpose counters, set with
-- 
2.23.0

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
  2019-11-04 16:33 [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof Jules Irenge
@ 2019-11-04 16:46 ` Greg KH
  2019-11-05 11:07   ` Jules Irenge
  2019-11-04 17:03 ` Ian Abbott
  1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-11-04 16:46 UTC (permalink / raw)
  To: Jules Irenge; +Cc: devel, outreachy-kernel, abbotti, linux-kernel

On Mon, Nov 04, 2019 at 04:33:31PM +0000, Jules Irenge wrote:
> Rewrite macro function with the GNU extension typeof
> to remove a possible side-effects of MACRO argument reuse "x".
>  - Problem could rise if arguments have different types
> and different use though.

You can not just get away with a potential problem by documenting it :)

You might have just broken this.  Why are you trying to "fix" something
that is not broken?

What is wrong with the code as-is?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
  2019-11-04 16:33 [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof Jules Irenge
  2019-11-04 16:46 ` Greg KH
@ 2019-11-04 17:03 ` Ian Abbott
  2019-11-04 20:46   ` Greg KH
  2019-11-05 11:00   ` Jules Irenge
  1 sibling, 2 replies; 6+ messages in thread
From: Ian Abbott @ 2019-11-04 17:03 UTC (permalink / raw)
  To: Jules Irenge, outreachy-kernel; +Cc: devel, gregkh, linux-kernel

On 04/11/2019 16:33, Jules Irenge wrote:
> Rewrite macro function with the GNU extension typeof
> to remove a possible side-effects of MACRO argument reuse "x".
>   - Problem could rise if arguments have different types
> and different use though.
> 
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> v1 - had no full commit log message, with changes not intended to be in the patch
> v2 - remove some changes not intended to be in this driver
>       include note of a potential problem
>   drivers/staging/comedi/comedi.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
> index 09a940066c0e..a57691a2e8d8 100644
> --- a/drivers/staging/comedi/comedi.h
> +++ b/drivers/staging/comedi/comedi.h
> @@ -1103,8 +1103,10 @@ enum ni_common_signal_names {
>   
>   /* *** END GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
>   
> -#define NI_USUAL_PFI_SELECT(x)	(((x) < 10) ? (0x1 + (x)) : (0xb + (x)))
> -#define NI_USUAL_RTSI_SELECT(x)	(((x) < 7) ? (0xb + (x)) : 0x1b)
> +#define NI_USUAL_PFI_SELECT(x)\
> +	({typeof(x) x_ = (x); (x_ < 10) ? (0x1 + x_) : (0xb + x_); })
> +#define NI_USUAL_RTSI_SELECT(x)\
> +	({typeof(x) x_ = (x); (x_ < 7) ? (0xb + x_) : 0x1b; })
>   
>   /*
>    * mode bits for NI general-purpose counters, set with
> 

I wasn't sure about this the first time around due to the use of GNU 
extensions in uapi header files, but I see there are a few, rare 
instances of this GNU extension elsewhere in other uapi headers (mainly 
in netfilter stuff), so I guess it's OK.  However, it  does mean that 
user code that uses these macros will no longer compile unless GNU 
extensions are enabled.

Does anyone know any "best practices" regarding use of GNU extensions in 
user header files under Linux?

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
  2019-11-04 17:03 ` Ian Abbott
@ 2019-11-04 20:46   ` Greg KH
  2019-11-05 11:00   ` Jules Irenge
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-11-04 20:46 UTC (permalink / raw)
  To: Ian Abbott; +Cc: devel, outreachy-kernel, Jules Irenge, linux-kernel

On Mon, Nov 04, 2019 at 05:03:04PM +0000, Ian Abbott wrote:
> On 04/11/2019 16:33, Jules Irenge wrote:
> > Rewrite macro function with the GNU extension typeof
> > to remove a possible side-effects of MACRO argument reuse "x".
> >   - Problem could rise if arguments have different types
> > and different use though.
> > 
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> > v1 - had no full commit log message, with changes not intended to be in the patch
> > v2 - remove some changes not intended to be in this driver
> >       include note of a potential problem
> >   drivers/staging/comedi/comedi.h | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
> > index 09a940066c0e..a57691a2e8d8 100644
> > --- a/drivers/staging/comedi/comedi.h
> > +++ b/drivers/staging/comedi/comedi.h
> > @@ -1103,8 +1103,10 @@ enum ni_common_signal_names {
> >   /* *** END GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
> > -#define NI_USUAL_PFI_SELECT(x)	(((x) < 10) ? (0x1 + (x)) : (0xb + (x)))
> > -#define NI_USUAL_RTSI_SELECT(x)	(((x) < 7) ? (0xb + (x)) : 0x1b)
> > +#define NI_USUAL_PFI_SELECT(x)\
> > +	({typeof(x) x_ = (x); (x_ < 10) ? (0x1 + x_) : (0xb + x_); })
> > +#define NI_USUAL_RTSI_SELECT(x)\
> > +	({typeof(x) x_ = (x); (x_ < 7) ? (0xb + x_) : 0x1b; })
> >   /*
> >    * mode bits for NI general-purpose counters, set with
> > 
> 
> I wasn't sure about this the first time around due to the use of GNU
> extensions in uapi header files, but I see there are a few, rare instances
> of this GNU extension elsewhere in other uapi headers (mainly in netfilter
> stuff), so I guess it's OK.  However, it  does mean that user code that uses
> these macros will no longer compile unless GNU extensions are enabled.
> 
> Does anyone know any "best practices" regarding use of GNU extensions in
> user header files under Linux?

We try to never do it if at all possible :)

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
  2019-11-04 17:03 ` Ian Abbott
  2019-11-04 20:46   ` Greg KH
@ 2019-11-05 11:00   ` Jules Irenge
  1 sibling, 0 replies; 6+ messages in thread
From: Jules Irenge @ 2019-11-05 11:00 UTC (permalink / raw)
  To: Ian Abbott; +Cc: devel, Jules Irenge, gregkh, linux-kernel, outreachy-kernel



On Mon, 4 Nov 2019, Ian Abbott wrote:

> On 04/11/2019 16:33, Jules Irenge wrote:
> > Rewrite macro function with the GNU extension typeof
> > to remove a possible side-effects of MACRO argument reuse "x".
> >   - Problem could rise if arguments have different types
> > and different use though.
> > 
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> > v1 - had no full commit log message, with changes not intended to be in the
> > patch
> > v2 - remove some changes not intended to be in this driver
> >       include note of a potential problem
> >   drivers/staging/comedi/comedi.h | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/comedi/comedi.h
> > b/drivers/staging/comedi/comedi.h
> > index 09a940066c0e..a57691a2e8d8 100644
> > --- a/drivers/staging/comedi/comedi.h
> > +++ b/drivers/staging/comedi/comedi.h
> > @@ -1103,8 +1103,10 @@ enum ni_common_signal_names {
> >   
> >   /* *** END GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
> >   
> > -#define NI_USUAL_PFI_SELECT(x)	(((x) < 10) ? (0x1 + (x)) : (0xb +
> > (x)))
> > -#define NI_USUAL_RTSI_SELECT(x)	(((x) < 7) ? (0xb + (x)) : 0x1b)
> > +#define NI_USUAL_PFI_SELECT(x)\
> > +	({typeof(x) x_ = (x); (x_ < 10) ? (0x1 + x_) : (0xb + x_); })
> > +#define NI_USUAL_RTSI_SELECT(x)\
> > +	({typeof(x) x_ = (x); (x_ < 7) ? (0xb + x_) : 0x1b; })
> >   
> >   /*
> >    * mode bits for NI general-purpose counters, set with
> > 
> 
> I wasn't sure about this the first time around due to the use of GNU
> extensions in uapi header files, but I see there are a few, rare instances of
> this GNU extension elsewhere in other uapi headers (mainly in netfilter
> stuff), so I guess it's OK.  However, it  does mean that user code that uses
> these macros will no longer compile unless GNU extensions are enabled.
> 
> Does anyone know any "best practices" regarding use of GNU extensions in user
> header files under Linux?
> 
> -- 
> -=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
> -=( MEV Ltd. is a company registered in England & Wales. )=-
> -=( Registered number: 02862268.  Registered address:    )=-
> -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
> 
> 
Apology, I misinterpreted the comments and send a second 
version without thinking much.
Please discard it unless you wish to try out.
Any way thanks for the feedback, I really appreciate I find it educative.
Kind regards,
Jules
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof
  2019-11-04 16:46 ` Greg KH
@ 2019-11-05 11:07   ` Jules Irenge
  0 siblings, 0 replies; 6+ messages in thread
From: Jules Irenge @ 2019-11-05 11:07 UTC (permalink / raw)
  To: Greg KH; +Cc: devel, outreachy-kernel, Jules Irenge, abbotti, linux-kernel



On Mon, 4 Nov 2019, Greg KH wrote:

> On Mon, Nov 04, 2019 at 04:33:31PM +0000, Jules Irenge wrote:
> > Rewrite macro function with the GNU extension typeof
> > to remove a possible side-effects of MACRO argument reuse "x".
> >  - Problem could rise if arguments have different types
> > and different use though.
> 
> You can not just get away with a potential problem by documenting it :)
> 
> You might have just broken this.  Why are you trying to "fix" something
> that is not broken?
> 
> What is wrong with the code as-is?
> 
> thanks,
> 
> greg k-h
> 
Again I made a lot of mistake. I will be more carefull.
Thanks for the feedback
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 16:33 [PATCH v2] staging: comedi: rewrite macro function with GNU extension typeof Jules Irenge
2019-11-04 16:46 ` Greg KH
2019-11-05 11:07   ` Jules Irenge
2019-11-04 17:03 ` Ian Abbott
2019-11-04 20:46   ` Greg KH
2019-11-05 11:00   ` Jules Irenge

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org
	public-inbox-index driverdev-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git