[KJ] Taking the Min and Max macro job

[KJ] Taking the Min and Max macro job

[Ahmed S. Darwish]

Hi list,

It seems that no one is assigned the Min and Max macro job. I'll take
it if no one seems interested.

Well, I hope this to become my last brain-dead patch series :).

Thanks,

-- 
Ahmed S. Darwish
http://darwish-07.blogspot.com
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Alexey Dobriyan]

On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> It seems that no one is assigned the Min and Max macro job. I'll take
> it if no one seems interested.
> 
> Well, I hope this to become my last brain-dead patch series :).

Be very very careful to not add integer overflows.

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Ahmed S. Darwish]

On Thu, Feb 08, 2007 at 09:16:53AM +0300, Alexey Dobriyan wrote:
> On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> > It seems that no one is assigned the Min and Max macro job. I'll take
> > it if no one seems interested.
> > 
> > Well, I hope this to become my last brain-dead patch series :).
> 
> Be very very careful to not add integer overflows.

Thanks, I'll keep that in mind.

-- 
Ahmed S. Darwish
http://darwish-07.blogspot.com
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 764 bytes --]

I had started the activity. changed the handcrafted MAX macro to *max* from
the kernel.h in the xdr4.c file ( no quite sure of the complete file name)

the max macro is applied in struct , i changed it and recompile failed.


On 2/8/07, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> > It seems that no one is assigned the Min and Max macro job. I'll take
> > it if no one seems interested.
> >
> > Well, I hope this to become my last brain-dead patch series :).
>
> Be very very careful to not add integer overflows.
>
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/kernel-janitors
>

[-- Attachment #1.2: Type: text/html, Size: 1247 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 809 bytes --]

Just let me know if you get thru the problem.

May be i am doing something wrong

Mehul Jani


On 2/8/07, Ahmed S. Darwish <darwish.07@gmail.com> wrote:
>
> On Thu, Feb 08, 2007 at 09:16:53AM +0300, Alexey Dobriyan wrote:
> > On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> > > It seems that no one is assigned the Min and Max macro job. I'll take
> > > it if no one seems interested.
> > >
> > > Well, I hope this to become my last brain-dead patch series :).
> >
> > Be very very careful to not add integer overflows.
>
> Thanks, I'll keep that in mind.
>
> --
> Ahmed S. Darwish
> http://darwish-07.blogspot.com
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/kernel-janitors
>

[-- Attachment #1.2: Type: text/html, Size: 1383 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Ahmed S. Darwish]

On Thu, Feb 08, 2007 at 02:59:41PM +0530, Mehul Jani wrote:
> On 2/8/07, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> >On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> >> It seems that no one is assigned the Min and Max macro job. I'll take
> >> it if no one seems interested.
> >>
> >> Well, I hope this to become my last brain-dead patch series :).
> >
> >Be very very careful to not add integer overflows.
> >
>
> I had started the activity. changed the handcrafted MAX macro to *max* from
> the kernel.h in the xdr4.c file ( no quite sure of the complete file name)
> 
> the max macro is applied in struct , i changed it and recompile failed.

Hi Mehul,

(Please use reply-to-all, and let the quoted text be on the top not on the 
bottom)

Will you continue the task for the whole tree ?.

-- 
Ahmed S. Darwish
http://darwish-07.blogspot.com
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 1086 bytes --]

I will do that. And let you know the progress .....

Thanks for pointing out for *reply all*

Mehul


On 2/8/07, Ahmed S. Darwish <darwish.07@gmail.com> wrote:
>
> On Thu, Feb 08, 2007 at 02:59:41PM +0530, Mehul Jani wrote:
> > On 2/8/07, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > >
> > >On Thu, Feb 08, 2007 at 01:51:45AM +0200, Ahmed S. Darwish wrote:
> > >> It seems that no one is assigned the Min and Max macro job. I'll take
> > >> it if no one seems interested.
> > >>
> > >> Well, I hope this to become my last brain-dead patch series :).
> > >
> > >Be very very careful to not add integer overflows.
> > >
> >
> > I had started the activity. changed the handcrafted MAX macro to *max*
> from
> > the kernel.h in the xdr4.c file ( no quite sure of the complete file
> name)
> >
> > the max macro is applied in struct , i changed it and recompile failed.
>
> Hi Mehul,
>
> (Please use reply-to-all, and let the quoted text be on the top not on the
> bottom)
>
> Will you continue the task for the whole tree ?.
>
> --
> Ahmed S. Darwish
> http://darwish-07.blogspot.com
>

[-- Attachment #1.2: Type: text/html, Size: 1637 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Ahmed S. Darwish]

On Thu, Feb 08, 2007 at 05:48:35PM +0530, Mehul Jani wrote:
> I will do that. And let you know the progress .....
>

As Alexey Doriyan advised me, 

Take a look at wikipedia `integer overflow' page and 
the links in the External links secition.

-- 
Ahmed S. Darwish
http://darwish-07.blogspot.com
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Robert P. J. Day]

On Thu, 8 Feb 2007, Mehul Jani wrote:

> I will do that. And let you know the progress .....
>
> Thanks for pointing out for *reply all*
>
> Mehul
>
>
> On 2/8/07, Ahmed S. Darwish <darwish.07@gmail.com> wrote:

...

don't top-post.  always put your reply *after* the text that you're
responding to.

*and* delete any text that isn't relevant to your reply.

rday

-- 
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 415 bytes --]

>
> > I will do that. And let you know the progress .....
> >
> > Thanks for pointing out for *reply all*
> >
> > Mehul
> >
> >
> > On 2/8/07, Ahmed S. Darwish <darwish.07@gmail.com> wrote:
>
> ...
>
> don't top-post.  always put your reply *after* the text that you're
> responding to.
>
> *and* delete any text that isn't relevant to your reply.
>
> rday



Make sense .. will take care from this time ;-)

Mehul

[-- Attachment #1.2: Type: text/html, Size: 721 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

>
>  > I will do that. And let you know the progress .....
> > >
> > > Thanks for pointing out for *reply all*
> > >
> > > Mehul
> > >
> > >
> > > On 2/8/07, Ahmed S. Darwish <darwish.07@gmail.com> wrote:
> >
> > ...
> >
> > don't top-post.  always put your reply *after* the text that you're
> > responding to.
> >
> > *and* delete any text that isn't relevant to your reply.
> >
> > rday
>
>
>
> Make sense .. will take care from this time ;-)
>
> Mehul
>


I tried the max_t macro specifying the type as int in the xdr4.c file . The
compile failed.
Any pointers

[-- Attachment #1.2: Type: text/html, Size: 1160 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Vignesh Babu BM]

On Fri, 2007-02-09 at 10:47 +0530, Mehul Jani wrote:
> xdr4.c

Mehul like I said...xdr4.c needs a simple macro that does not check type
at all. 

If my understanding of the min/max use is correct then they use default
type checking and min_t/max_t use the type that 'we' specify...so end
result-they are both bound to type-meaning they would both have
statements in expression-resulting in compilation failure as xdr*.c uses
them inside struct initializations.

But the macro that would work in xdr*.c would look something like the
below:

#define min_s(x,y) ((x)>(y)?(x):(y))

But with the above we need to make sure that x and y are not expressions
if we dont want them to be evaluated twice and hence cause other
probs ;)

So either we modify min/max in kernel.h - this means a lot more cleanup
or add another macro say min_s/max_s --- s for simple :) 

Unless we don't do that, it may not be possible to cleanup files like
xdr*.c

-- 
Regards,  
Vignesh Babu BM  
_____________________________________________________________  
"Why is it that every time I'm with you, makes me believe in magic?"

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Richard Knutsson]

Vignesh Babu BM wrote:
> Mehul like I said...xdr4.c needs a simple macro that does not check type
> at all. 
>
> If my understanding of the min/max use is correct then they use default
> type checking and min_t/max_t use the type that 'we' specify...so end
> result-they are both bound to type-meaning they would both have
> statements in expression-resulting in compilation failure as xdr*.c uses
> them inside struct initializations.
>
> But the macro that would work in xdr*.c would look something like the
> below:
>
> #define min_s(x,y) ((x)>(y)?(x):(y))
>
> But with the above we need to make sure that x and y are not expressions
> if we dont want them to be evaluated twice and hence cause other
> probs ;)
>
> So either we modify min/max in kernel.h - this means a lot more cleanup
> or add another macro say min_s/max_s --- s for simple :) 
>
> Unless we don't do that, it may not be possible to cleanup files like
> xdr*.c
>   
What about __min/__max? At least I usually read the '__'-prefix as 
something that is a subset of something (and most likely less secure).
I wrote about this in "[KJ] [RFC] Regarding min/max" (07-02-03) bit 
since I was not able to find an example I took it as a mistake on my part.

Richard Knutsson

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Vignesh Babu BM]

On Fri, 2007-02-09 at 08:15 +0100, Richard Knutsson wrote:
> Vignesh Babu BM wrote:
> > Mehul like I said...xdr4.c needs a simple macro that does not check type
> > at all. 
> >
> > But the macro that would work in xdr*.c would look something like the
> > below:
> >
> > #define min_s(x,y) ((x)>(y)?(x):(y))
> >
> >
> > So either we modify min/max in kernel.h - this means a lot more cleanup
> > or add another macro say min_s/max_s --- s for simple :) 
> >
> > Unless we don't do that, it may not be possible to cleanup files like
> > xdr*.c
> >   
> What about __min/__max? At least I usually read the '__'-prefix as 
> something that is a subset of something (and most likely less secure).
> I wrote about this in "[KJ] [RFC] Regarding min/max" (07-02-03) bit 
> since I was not able to find an example I took it as a mistake on my part.
> 
> Richard Knutsson
> 

Richard are you suggesting a name like __min/__max? That would be great.
-- 
Regards,  
Vignesh Babu BM  
_____________________________________________________________  
"Why is it that every time I'm with you, makes me believe in magic?"

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Vignesh Babu BM]

On Fri, 2007-02-09 at 08:15 +0100, Richard Knutsson wrote:
> Vignesh Babu BM wrote:
> > Mehul like I said...xdr4.c needs a simple macro that does not check type
> > at all. 
> >
> > But the macro that would work in xdr*.c would look something like the
> > below:
> >
> > #define min_s(x,y) ((x)>(y)?(x):(y))
> >
> > So either we modify min/max in kernel.h - this means a lot more cleanup
> > or add another macro say min_s/max_s --- s for simple :) 
> >
> > Unless we don't do that, it may not be possible to cleanup files like
> > xdr*.c
> >   
> What about __min/__max? At least I usually read the '__'-prefix as 
> something that is a subset of something (and most likely less secure).
> I wrote about this in "[KJ] [RFC] Regarding min/max" (07-02-03) bit 
> since I was not able to find an example I took it as a mistake on my part.
> 
> Richard Knutsson
> 

Just went through the archives....

        I tried to recreate the compile-failure (was some time ago since
        it 
        happened) when using min()/max() in a struct but it happily compiled it 
        so (thankfully) you are correct, just need to use min()/max(). :)
        
        Richard Knutsson
        
Do you mean to say that you were able to compile when the macros were used inside struct?
Im still not able to get that to happen...

I tried that change in linux-2.6/fs/lockd/mon.c

static struct rpc_procinfo	nsm_procedures[] = {
[SM_MON] = {
		.p_proc		= SM_MON,
		.p_encode	= (kxdrproc_t) xdr_encode_mon,
		.p_decode	= (kxdrproc_t) xdr_decode_stat_res,
		.p_bufsiz	= max(SM_mon_sz, SM_monres_sz) << 2,
		.p_statidx	= SM_MON,
		.p_name		= "MONITOR",
	},
[SM_UNMON] = {
		.p_proc		= SM_UNMON,
		.p_encode	= (kxdrproc_t) xdr_encode_unmon,
		.p_decode	= (kxdrproc_t) xdr_decode_stat,
		.p_bufsiz	= max(SM_mon_id_sz, SM_unmonres_sz) << 2,
		.p_statidx	= SM_UNMON,
		.p_name		= "UNMONITOR",
	},
};
-- 
Regards,  
Vignesh Babu BM  
_____________________________________________________________  
"Why is it that every time I'm with you, makes me believe in magic?"
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Richard Knutsson]

Vignesh Babu BM wrote:
> Just went through the archives....
>
>         I tried to recreate the compile-failure (was some time ago since
>         it 
>         happened) when using min()/max() in a struct but it happily compiled it 
>         so (thankfully) you are correct, just need to use min()/max(). :)
>         
>         Richard Knutsson
>         
> Do you mean to say that you were able to compile when the macros were used inside struct?
> Im still not able to get that to happen...
>
> I tried that change in linux-2.6/fs/lockd/mon.c
>
> static struct rpc_procinfo	nsm_procedures[] = {
> [SM_MON] = {
> 		.p_proc		= SM_MON,
> 		.p_encode	= (kxdrproc_t) xdr_encode_mon,
> 		.p_decode	= (kxdrproc_t) xdr_decode_stat_res,
> 		.p_bufsiz	= max(SM_mon_sz, SM_monres_sz) << 2,
> 		.p_statidx	= SM_MON,
> 		.p_name		= "MONITOR",
> 	},
> [SM_UNMON] = {
> 		.p_proc		= SM_UNMON,
> 		.p_encode	= (kxdrproc_t) xdr_encode_unmon,
> 		.p_decode	= (kxdrproc_t) xdr_decode_stat,
> 		.p_bufsiz	= max(SM_mon_id_sz, SM_unmonres_sz) << 2,
> 		.p_statidx	= SM_UNMON,
> 		.p_name		= "UNMONITOR",
> 	},
> };
>   
No, I "recreated" it by making a small test-file where I used 
min()/max() in a struct, and it worked without a pip from the compiler.
Can not say I'm sure what the difference were, but xdr4.c and mon.c 
didn't seem to like it.

What do you say of something like this below? (It is not tested)

(Signed-off-by:) Richard Knutsson
---

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 63fb18d..0533b2d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -252,6 +252,14 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
 #error "Please fix asm/byteorder.h"
 #endif /* __LITTLE_ENDIAN */
 
+
+/*
+ * __min()/__max() is unsafe.
+ * Use min()/max() when available.
+ */
+#define __min(x,y)	((x) < (y) ? (x) : (y))
+#define __max(x,y)	((x) > (y) ? (x) : (y))
+
 /*
  * min()/max() macros that also do
  * strict type-checking.. See the
@@ -261,13 +269,13 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
 	typeof(x) _x = (x);	\
 	typeof(y) _y = (y);	\
 	(void) (&_x = &_y);		\
-	_x < _y ? _x : _y; })
+	__min(_x, _y); })
 
 #define max(x,y) ({ \
 	typeof(x) _x = (x);	\
 	typeof(y) _y = (y);	\
 	(void) (&_x = &_y);		\
-	_x > _y ? _x : _y; })
+	__max(_x, _y); })
 
 /*
  * ..and if you can't take the strict
@@ -276,9 +284,9 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
  * Or not use min/max at all, of course.
  */
 #define min_t(type,x,y) \
-	({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
+	({ type __x = (x); type __y = (y); __min(__x, __y); })
 #define max_t(type,x,y) \
-	({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
+	({ type __x = (x); type __y = (y); __max(__x, __y); })
 
 
 /**


_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Vignesh Babu BM]

On Fri, 2007-02-09 at 09:55 +0100, Richard Knutsson wrote:
> Vignesh Babu BM wrote:
> > Just went through the archives....
> >
> >         I tried to recreate the compile-failure (was some time ago since
> >         it 
> >         happened) when using min()/max() in a struct but it happily compiled it 
> >         so (thankfully) you are correct, just need to use min()/max(). :)
> >         
> >         Richard Knutsson
> >         
> > Do you mean to say that you were able to compile when the macros were used inside struct?
> > Im still not able to get that to happen...
> >
> > I tried that change in linux-2.6/fs/lockd/mon.c
> >
> > static struct rpc_procinfo	nsm_procedures[] = {
> > [SM_MON] = {
> > 		.p_proc		= SM_MON,
> > 		.p_encode	= (kxdrproc_t) xdr_encode_mon,
> > 		.p_decode	= (kxdrproc_t) xdr_decode_stat_res,
> > 		.p_bufsiz	= max(SM_mon_sz, SM_monres_sz) << 2,
> > 		.p_statidx	= SM_MON,
> > 		.p_name		= "MONITOR",
> > 	},
> > [SM_UNMON] = {
> > 		.p_proc		= SM_UNMON,
> > 		.p_encode	= (kxdrproc_t) xdr_encode_unmon,
> > 		.p_decode	= (kxdrproc_t) xdr_decode_stat,
> > 		.p_bufsiz	= max(SM_mon_id_sz, SM_unmonres_sz) << 2,
> > 		.p_statidx	= SM_UNMON,
> > 		.p_name		= "UNMONITOR",
> > 	},
> > };
> >   
> No, I "recreated" it by making a small test-file where I used 
> min()/max() in a struct, and it worked without a pip from the compiler.
> Can not say I'm sure what the difference were, but xdr4.c and mon.c 
> didn't seem to like it.
> 
> What do you say of something like this below? (It is not tested)
> 
> (Signed-off-by:) Richard Knutsson
> ---
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 63fb18d..0533b2d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -252,6 +252,14 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
>  #error "Please fix asm/byteorder.h"
>  #endif /* __LITTLE_ENDIAN */
>  
> +
> +/*
> + * __min()/__max() is unsafe.
> + * Use min()/max() when available.
> + */
> +#define __min(x,y)	((x) < (y) ? (x) : (y))
> +#define __max(x,y)	((x) > (y) ? (x) : (y))
> +
>  /*
>   * min()/max() macros that also do
>   * strict type-checking.. See the
> @@ -261,13 +269,13 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
>  	typeof(x) _x = (x);	\
>  	typeof(y) _y = (y);	\
>  	(void) (&_x = &_y);		\
> -	_x < _y ? _x : _y; })
> +	__min(_x, _y); })
>  
>  #define max(x,y) ({ \
>  	typeof(x) _x = (x);	\
>  	typeof(y) _y = (y);	\
>  	(void) (&_x = &_y);		\
> -	_x > _y ? _x : _y; })
> +	__max(_x, _y); })
>  
>  /*
>   * ..and if you can't take the strict
> @@ -276,9 +284,9 @@ static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char *
>   * Or not use min/max at all, of course.
>   */
>  #define min_t(type,x,y) \
> -	({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
> +	({ type __x = (x); type __y = (y); __min(__x, __y); })
>  #define max_t(type,x,y) \
> -	({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
> +	({ type __x = (x); type __y = (y); __max(__x, __y); })
>  
> 
>  /**
> 

The patch works fine...the compilation went thru. I hope this gets
accepted.

-- 
Regards,  
Vignesh Babu BM  
_____________________________________________________________  
"Why is it that every time I'm with you, makes me believe in magic?"
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Jaco Kroon]

[-- Attachment #1.1: Type: text/plain, Size: 1010 bytes --]

Vignesh Babu BM wrote:
> On Fri, 2007-02-09 at 09:55 +0100, Richard Knutsson wrote:
> 
>>Vignesh Babu BM wrote:
>>
>>>Just went through the archives....
>>>
>>>        I tried to recreate the compile-failure (was some time ago since
>>>        it 
>>>        happened) when using min()/max() in a struct but it happily compiled it 
>>>        so (thankfully) you are correct, just need to use min()/max(). :)
>>>        
>>>        Richard Knutsson
>>>        
>>>Do you mean to say that you were able to compile when the macros were used inside struct?
>>>Im still not able to get that to happen...
>>>
>>>I tried that change in linux-2.6/fs/lockd/mon.c
>>>
>>>static struct rpc_procinfo	nsm_procedures[] = {
>>>[SM_MON] = {
>>>		.p_proc		= SM_MON,
>>>		.p_encode	= (kxdrproc_t) xdr_encode_mon,
>>>		.p_decode	= (kxdrproc_t) xdr_decode_stat_res,
>>>		.p_bufsiz	= max(SM_mon_sz, SM_monres_sz) << 2,

Will work since both SM_mon_sz and SM_monres_sz are compile-time
constants, not true?

[ ... snip ... ]

Jaco

[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3233 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 1297 bytes --]

>
> >>>Just went through the archives....
> >>>
> >>>        I tried to recreate the compile-failure (was some time ago
> since
> >>>        it
> >>>        happened) when using min()/max() in a struct but it happily
> compiled it
> >>>        so (thankfully) you are correct, just need to use min()/max().
> :)
> >>>
> >>>        Richard Knutsson
> >>>
> >>>Do you mean to say that you were able to compile when the macros were
> used inside struct?
> >>>Im still not able to get that to happen...
> >>>
> >>>I tried that change in linux-2.6/fs/lockd/mon.c
> >>>
> >>>static struct rpc_procinfo   nsm_procedures[] = {
> >>>[SM_MON] = {
> >>>             .p_proc         = SM_MON,
> >>>             .p_encode       = (kxdrproc_t) xdr_encode_mon,
> >>>             .p_decode       = (kxdrproc_t) xdr_decode_stat_res,
> >>>             .p_bufsiz       = max(SM_mon_sz, SM_monres_sz) << 2,
>
> Will work since both SM_mon_sz and SM_monres_sz are compile-time
> constants, not true?
>
> [ ... snip ... ]


So...

The patch actually modifies the macro's last line in kernel.h

-       _x < _y ? _x : _y; })
+       __min(_x, _y); })

But what I am unable to understand is why is the compiler unhappy with
* _x < _y ? _x : _y; })*

and happy with new macro been called internally *__min(_x,_y);*

Mehul

[-- Attachment #1.2: Type: text/html, Size: 2469 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Richard Knutsson]

Mehul Jani wrote:
>
>
>     >>>Just went through the archives....
>     >>>
>     >>>        I tried to recreate the compile-failure (was some time
>     ago since
>     >>>        it
>     >>>        happened) when using min()/max() in a struct but it
>     happily compiled it
>     >>>        so (thankfully) you are correct, just need to use
>     min()/max(). :)
>     >>>
>     >>>        Richard Knutsson
>     >>>
>     >>>Do you mean to say that you were able to compile when the
>     macros were used inside struct?
>     >>>Im still not able to get that to happen...
>     >>>
>     >>>I tried that change in linux-2.6/fs/lockd/mon.c
>     >>>
>     >>>static struct rpc_procinfo   nsm_procedures[] = {
>     >>>[SM_MON] = {
>     >>>             .p_proc         = SM_MON,
>     >>>             .p_encode       = (kxdrproc_t) xdr_encode_mon,
>     >>>             .p_decode       = (kxdrproc_t) xdr_decode_stat_res,
>     >>>             .p_bufsiz       = max(SM_mon_sz, SM_monres_sz) << 2,
>
>     Will work since both SM_mon_sz and SM_monres_sz are compile-time
>     constants, not true?
>
>     [ ... snip ... ]
>
>  
> So...
>  
> The patch actually modifies the macro's last line in kernel.h
>  
> -       _x < _y ? _x : _y; })
> +       __min(_x, _y); })
>  
> But what I am unable to understand is why is the compiler unhappy with
> * _x < _y ? _x : _y; })*
>  
> and happy with new macro been called internally *__min(_x,_y);*
Oh, it is still unhappy with min()/max() in the above struct. You will 
have to use __min()/__max() directly to please the compiler.
I just modified the other macros since I thought it was unnecessary (and 
error-prone) to define the "algorithm" multiple times + it makes the 
logical statement that __min()/__max() are the unsecure macros and by 
adding type-checking we get the secure min()/max().

Richard Knutsson

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Richard Knutsson]

Mehul Jani wrote:
>
>     >>>I tried that change in linux-2.6/fs/lockd/mon.c
>     >>>
>     >>>static struct rpc_procinfo   nsm_procedures[] = {
>     >>>[SM_MON] = {
>     >>>             .p_proc         = SM_MON,
>     >>>             .p_encode       = (kxdrproc_t) xdr_encode_mon,
>     >>>             .p_decode       = (kxdrproc_t) xdr_decode_stat_res,
>     >>>             .p_bufsiz       = max(SM_mon_sz, SM_monres_sz) << 2,
>
>     Will work since both SM_mon_sz and SM_monres_sz are compile-time
>     constants, not true?
>
>     [ ... snip ... ]
>
>  
> So...
>  
> The patch actually modifies the macro's last line in kernel.h
>  
> -       _x < _y ? _x : _y; })
> +       __min(_x, _y); })
>  
> But what I am unable to understand is why is the compiler unhappy with
> * _x < _y ? _x : _y; })*
>  
> and happy with new macro been called internally *__min(_x,_y);*
Ok, _now_ I see. I used the xdr4.c but mon.c seems happy with the new 
max() (as you asked). Can only say: dunno. It should not make a difference.

Richard Knutsson

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 1409 bytes --]

> >     >>>I tried that change in linux-2.6/fs/lockd/mon.c
> >     >>>
> >     >>>static struct rpc_procinfo   nsm_procedures[] = {
> >     >>>[SM_MON] = {
> >     >>>             .p_proc         = SM_MON,
> >     >>>             .p_encode       = (kxdrproc_t) xdr_encode_mon,
> >     >>>             .p_decode       = (kxdrproc_t) xdr_decode_stat_res,
> >     >>>             .p_bufsiz       = max(SM_mon_sz, SM_monres_sz) << 2,
> >
> >     Will work since both SM_mon_sz and SM_monres_sz are compile-time
> >     constants, not true?
> >
> >     [ ... snip ... ]
> >
> >
> > So...
> >
> > The patch actually modifies the macro's last line in kernel.h
> >
> > -       _x < _y ? _x : _y; })
> > +       __min(_x, _y); })
> >
> > But what I am unable to understand is why is the compiler unhappy with
> > * _x < _y ? _x : _y; })*
> >
> > and happy with new macro been called internally *__min(_x,_y);*
> Ok, _now_ I see. I used the xdr4.c but mon.c seems happy with the new
> max() (as you asked). Can only say: dunno. It should not make a
> difference.
>
> Richard Knutsson


Richard,

I believe Vignesh had confirmed the patch.....

So now this is going to be crazy we have 3 pairs of min/max to deal with

min()/max()
min_t()/max_t()
__min()/__max()

Can the first one be considered *redundant*

I was wondering that from the very first. calling MACRO(MACRO()) somehow is
dirty....Any suggestions !!

Mehul

[-- Attachment #1.2: Type: text/html, Size: 2634 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Richard Knutsson]

Mehul Jani wrote: 
> I believe Vignesh had confirmed the patch.....
>  
> So now this is going to be crazy we have 3 pairs of min/max to deal with
>  
> min()/max()
> min_t()/max_t()
> __min()/__max()
>  
> Can the first one be considered *redundant*
min()/max()? That is our crown-jewel. I think the order above is then 
one how to implement min/max. Use ex min() when possible. If min() does 
not work, use min_t(). If min_t() does not work, use __min() (if it will 
be accepted, that is). But you have to be more careful when using 
__min() since it can have side-effects.
> I was wondering that from the very first. calling MACRO(MACRO()) 
> somehow is dirty....Any suggestions !! 
Why? Calling function1(function2()) is ok (right?) and also when dealing 
with pci-ids you use macros in macros. And the way __min() is in min() 
and min_t() is also just common practice to reduce multiple declarations.

Richard Knutsson

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Vignesh Babu BM]

On Fri, 2007-02-09 at 23:28 +0100, Richard Knutsson wrote:
> Mehul Jani wrote: 
> > I believe Vignesh had confirmed the patch.....
> >  
> > So now this is going to be crazy we have 3 pairs of min/max to deal with
> >  
> > min()/max()
> > min_t()/max_t()
> > __min()/__max()
> >  
> > Can the first one be considered *redundant*
> min()/max()? That is our crown-jewel. I think the order above is then 
> one how to implement min/max. Use ex min() when possible. If min() does 
> not work, use min_t(). If min_t() does not work, use __min() (if it will 
> be accepted, that is). But you have to be more careful when using 
> __min() since it can have side-effects.
> > I was wondering that from the very first. calling MACRO(MACRO()) 
> > somehow is dirty....Any suggestions !! 
> Why? Calling function1(function2()) is ok (right?) and also when dealing 
> with pci-ids you use macros in macros. And the way __min() is in min() 
> and min_t() is also just common practice to reduce multiple declarations.
> 
> Richard Knutsson
> 
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/kernel-janitors

Mehul I have tested the patch-compiles cleanly.
-- 
Regards,  
Vignesh Babu BM  
_____________________________________________________________  
"Why is it that every time I'm with you, makes me believe in magic?"
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Mehul Jani]

[-- Attachment #1.1: Type: text/plain, Size: 1180 bytes --]

>
>
> > > I believe Vignesh had confirmed the patch.....
> > >
> > > So now this is going to be crazy we have 3 pairs of min/max to deal
> with
> > >
> > > min()/max()
> > > min_t()/max_t()
> > > __min()/__max()
> > >
> > > Can the first one be considered *redundant*
> > min()/max()? That is our crown-jewel. I think the order above is then
> > one how to implement min/max. Use ex min() when possible. If min() does
> > not work, use min_t(). If min_t() does not work, use __min() (if it will
> > be accepted, that is). But you have to be more careful when using
> > __min() since it can have side-effects.
> > > I was wondering that from the very first. calling MACRO(MACRO())
> > > somehow is dirty....Any suggestions !!
> > Why? Calling function1(function2()) is ok (right?) and also when dealing
> > with pci-ids you use macros in macros. And the way __min() is in min()
> > and min_t() is also just common practice to reduce multiple
> declarations.
> >


Some how it did not look good , but you are right ....looks fine and clearly
i understood

Mehul I have tested the patch-compiles cleanly.


Great !!!

Will you patch the complete patching the complete tree ?

-Mehul

[-- Attachment #1.2: Type: text/html, Size: 1662 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Taking the Min and Max macro job

[Robert P. J. Day]

On Sat, 10 Feb 2007, Mehul Jani wrote:

> Mehul I have tested the patch-compiles cleanly.
>
>
> Great !!!
>
> Will you patch the complete patching the complete tree ?

i only vaguely followed this discussion.  could someone summarize what
this was all about on the wiki page if they have time?

rday

-- 
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors