linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 2.4.9 does not compile
@ 2001-08-16 21:11 f5ibh
  2001-08-16 21:41 ` 2.4.9 does not compile [PATCH] tpepper
                   ` (6 more replies)
  0 siblings, 7 replies; 56+ messages in thread
From: f5ibh @ 2001-08-16 21:11 UTC (permalink / raw)
  To: linux-kernel

Hi !

I've the following error messages :

gcc -D__KERNEL__ -I/usr/src/kernel-sources-2.4.9/include -Wall
-Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer
-fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2 -march=k6
-DMODULE -DMODVERSIONS -include
/usr/src/kernel-sources-2.4.9/include/linux/modversions.h
-DNTFS_VERSION=\"1.1.16\"   -c -o unistr.o unistr.c
unistr.c: In function `ntfs_collate_names':
unistr.c:99: warning: implicit declaration of function `min'
unistr.c:99: parse error before `unsigned'
unistr.c:99: parse error before `)'
unistr.c:97: warning: `c1' might be used uninitialized in this function
unistr.c: At top level:
unistr.c:118: parse error before `if'
unistr.c:123: warning: type defaults to `int' in declaration of `c1'
unistr.c:123: `name1' undeclared here (not in a function)
unistr.c:123: warning: data definition has no type or storage class
unistr.c:124: parse error before `if'
make[3]: *** [unistr.o] Erreur 1
make[3]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs/ntfs'
make[2]: *** [_modsubdir_ntfs] Erreur 2
make[2]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs'
make[1]: *** [_mod_fs] Erreur 2
make[1]: Leaving directory `/usr/src/kernel-sources-2.4.9'
make: *** [stamp-build] Erreur 2


gcc is :
Reading specs from /usr/lib/gcc-lib/i386-linux/2.95.4/specs
gcc version 2.95.4 20010319 (Debian prerelease)

------
Regards
		Jean-Luc

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
@ 2001-08-16 21:41 ` tpepper
  2001-08-16 22:10 ` 2.4.9 does not compile [better PATCH] tpepper
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 56+ messages in thread
From: tpepper @ 2001-08-16 21:41 UTC (permalink / raw)
  To: f5ibh; +Cc: linux-kernel

Confirmed here.  Looks like a pretty obvious goof to me.  Does the following
fix it for you?

--- linux-2.4.9/fs/ntfs/unistr.c.orig   Thu Aug 16 14:35:03 2001
+++ linux-2.4.9/fs/ntfs/unistr.c        Thu Aug 16 14:35:15 2001
@@ -96,7 +96,7 @@
        __u32 cnt;
        wchar_t c1, c2;
 
-       for (cnt = 0; cnt < min(unsigned int, name1_len, name2_len); ++cnt)
+       for (cnt = 0; cnt < min(name1_len, name2_len); ++cnt)
        {
                c1 = le16_to_cpu(*name1++);
                c2 = le16_to_cpu(*name2++);

> unistr.c: In function `ntfs_collate_names':
> unistr.c:99: warning: implicit declaration of function `min'
> unistr.c:99: parse error before `unsigned'
> unistr.c:99: parse error before `)'
> unistr.c:97: warning: `c1' might be used uninitialized in this function
> unistr.c: At top level:
> unistr.c:118: parse error before `if'
> unistr.c:123: warning: type defaults to `int' in declaration of `c1'
> unistr.c:123: `name1' undeclared here (not in a function)
> unistr.c:123: warning: data definition has no type or storage class
> unistr.c:124: parse error before `if'
> make[3]: *** [unistr.o] Erreur 1
> make[3]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs/ntfs'
> make[2]: *** [_modsubdir_ntfs] Erreur 2
> make[2]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs'
> make[1]: *** [_mod_fs] Erreur 2
> make[1]: Leaving directory `/usr/src/kernel-sources-2.4.9'
> make: *** [stamp-build] Erreur 2

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

* Re: 2.4.9 does not compile [better PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
  2001-08-16 21:41 ` 2.4.9 does not compile [PATCH] tpepper
@ 2001-08-16 22:10 ` tpepper
  2001-08-16 22:21 ` 2.4.9 does not compile [PATCH] Anton Altaparmakov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 56+ messages in thread
From: tpepper @ 2001-08-16 22:10 UTC (permalink / raw)
  To: f5ibh; +Cc: linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 2057 bytes --]

It looks like they are actually using a 3 argument min() function in the ntfs
code.  It seems happy with these three arg's and an include added so this patch is pr÷bably preferable:

diff -Naur linux-2.4.9/fs/ntfs/unistr.c.orig linux-2.4.9/fs/ntfs/unistr.c
--- linux-2.4.9/fs/ntfs/unistr.c.orig   Thu Aug 16 14:35:03 2001
+++ linux-2.4.9/fs/ntfs/unistr.c        Thu Aug 16 15:07:54 2001
@@ -24,6 +24,7 @@
 #include <linux/string.h>
 #include <asm/byteorder.h>
 
+#include "ntfstypes.h"
 #include "unistr.h"
 #include "macros.h"
 
@@ -96,7 +97,7 @@
        __u32 cnt;
        wchar_t c1, c2;
 
-       for (cnt = 0; cnt < min(unsigned int, name1_len, name2_len); ++cnt)
+       for (cnt = 0; cnt < min(__u32, name1_len, name2_len); ++cnt)
        {
                c1 = le16_to_cpu(*name1++);
                c2 = le16_to_cpu(*name2++);

Tim

--
*********************************************************
*  tpepper@vato dot org             * Venimus, Vidimus, *
*  http://www.vato.org/~tpepper     * Dolavimus         *
*********************************************************


On Thu 16 Aug at 23:11:40 +0200 f5ibh@db0bm.ampr.org done said:

> unistr.c: In function `ntfs_collate_names':
> unistr.c:99: warning: implicit declaration of function `min'
> unistr.c:99: parse error before `unsigned'
> unistr.c:99: parse error before `)'
> unistr.c:97: warning: `c1' might be used uninitialized in this function
> unistr.c: At top level:
> unistr.c:118: parse error before `if'
> unistr.c:123: warning: type defaults to `int' in declaration of `c1'
> unistr.c:123: `name1' undeclared here (not in a function)
> unistr.c:123: warning: data definition has no type or storage class
> unistr.c:124: parse error before `if'
> make[3]: *** [unistr.o] Erreur 1
> make[3]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs/ntfs'
> make[2]: *** [_modsubdir_ntfs] Erreur 2
> make[2]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs'
> make[1]: *** [_mod_fs] Erreur 2
> make[1]: Leaving directory `/usr/src/kernel-sources-2.4.9'
> make: *** [stamp-build] Erreur 2

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
  2001-08-16 21:41 ` 2.4.9 does not compile [PATCH] tpepper
  2001-08-16 22:10 ` 2.4.9 does not compile [better PATCH] tpepper
@ 2001-08-16 22:21 ` Anton Altaparmakov
  2001-08-16 22:28 ` 2.4.9 does not compile [better PATCH] Anton Altaparmakov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-16 22:21 UTC (permalink / raw)
  To: tpepper; +Cc: f5ibh, linux-kernel

Wrong fix. Jules Colding already sent correct one (an hour ago or so).

At 22:41 16/08/2001, tpepper@vato.org wrote:
>Confirmed here.  Looks like a pretty obvious goof to me.

Yes. Someone forgot to add kernel.h include after they removed the ntfs 
specific macros... Grr...

>   Does the following fix it for you?
>
>--- linux-2.4.9/fs/ntfs/unistr.c.orig   Thu Aug 16 14:35:03 2001
>+++ linux-2.4.9/fs/ntfs/unistr.c        Thu Aug 16 14:35:15 2001
>@@ -96,7 +96,7 @@
>         __u32 cnt;
>         wchar_t c1, c2;
>
>-       for (cnt = 0; cnt < min(unsigned int, name1_len, name2_len); ++cnt)
>+       for (cnt = 0; cnt < min(name1_len, name2_len); ++cnt)

This won't fix it because the min macro still will not exist.

Anton

>         {
>                 c1 = le16_to_cpu(*name1++);
>                 c2 = le16_to_cpu(*name2++);
>
> > unistr.c: In function `ntfs_collate_names':
> > unistr.c:99: warning: implicit declaration of function `min'
> > unistr.c:99: parse error before `unsigned'
> > unistr.c:99: parse error before `)'
> > unistr.c:97: warning: `c1' might be used uninitialized in this function
> > unistr.c: At top level:
> > unistr.c:118: parse error before `if'
> > unistr.c:123: warning: type defaults to `int' in declaration of `c1'
> > unistr.c:123: `name1' undeclared here (not in a function)
> > unistr.c:123: warning: data definition has no type or storage class
> > unistr.c:124: parse error before `if'
> > make[3]: *** [unistr.o] Erreur 1
> > make[3]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs/ntfs'
> > make[2]: *** [_modsubdir_ntfs] Erreur 2
> > make[2]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs'
> > make[1]: *** [_mod_fs] Erreur 2
> > make[1]: Leaving directory `/usr/src/kernel-sources-2.4.9'
> > make: *** [stamp-build] Erreur 2
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [better PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
                   ` (2 preceding siblings ...)
  2001-08-16 22:21 ` 2.4.9 does not compile [PATCH] Anton Altaparmakov
@ 2001-08-16 22:28 ` Anton Altaparmakov
  2001-08-17  4:23   ` Jeff Chua
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-16 22:28 UTC (permalink / raw)
  To: tpepper; +Cc: f5ibh, linux-kernel

At 23:10 16/08/2001, tpepper@vato.org wrote:
>It looks like they are actually using a 3 argument min() function in the ntfs
>code.

No, we are not using a 3 argument min() macro in ntfs code. At least we 
weren't until someone decided to replace all existing max/min macros from 
the entire kernel with this new 3 argument version and in the process 
forgot to add the kernel.h include thus breaking ntfs compilation...

Oh well, we will have to live with it until Linus comes back.

>It seems happy with these three arg's and an include added so this patch 
>is pr÷bably preferable:
>
>diff -Naur linux-2.4.9/fs/ntfs/unistr.c.orig linux-2.4.9/fs/ntfs/unistr.c
>--- linux-2.4.9/fs/ntfs/unistr.c.orig   Thu Aug 16 14:35:03 2001
>+++ linux-2.4.9/fs/ntfs/unistr.c        Thu Aug 16 15:07:54 2001
>@@ -24,6 +24,7 @@
>  #include <linux/string.h>
>  #include <asm/byteorder.h>
>
>+#include "ntfstypes.h"

that should be +#include <linux/kernel.h>

Best regards,

Anton

>  #include "unistr.h"
>  #include "macros.h"
>
>@@ -96,7 +97,7 @@
>         __u32 cnt;
>         wchar_t c1, c2;
>
>-       for (cnt = 0; cnt < min(unsigned int, name1_len, name2_len); ++cnt)
>+       for (cnt = 0; cnt < min(__u32, name1_len, name2_len); ++cnt)
>         {
>                 c1 = le16_to_cpu(*name1++);
>                 c2 = le16_to_cpu(*name2++);
>
>Tim
>
>--
>*********************************************************
>*  tpepper@vato dot org             * Venimus, Vidimus, *
>*  http://www.vato.org/~tpepper     * Dolavimus         *
>*********************************************************
>
>
>On Thu 16 Aug at 23:11:40 +0200 f5ibh@db0bm.ampr.org done said:
>
> > unistr.c: In function `ntfs_collate_names':
> > unistr.c:99: warning: implicit declaration of function `min'
> > unistr.c:99: parse error before `unsigned'
> > unistr.c:99: parse error before `)'
> > unistr.c:97: warning: `c1' might be used uninitialized in this function
> > unistr.c: At top level:
> > unistr.c:118: parse error before `if'
> > unistr.c:123: warning: type defaults to `int' in declaration of `c1'
> > unistr.c:123: `name1' undeclared here (not in a function)
> > unistr.c:123: warning: data definition has no type or storage class
> > unistr.c:124: parse error before `if'
> > make[3]: *** [unistr.o] Erreur 1
> > make[3]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs/ntfs'
> > make[2]: *** [_modsubdir_ntfs] Erreur 2
> > make[2]: Leaving directory `/usr/src/kernel-sources-2.4.9/fs'
> > make[1]: *** [_mod_fs] Erreur 2
> > make[1]: Leaving directory `/usr/src/kernel-sources-2.4.9'
> > make: *** [stamp-build] Erreur 2
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
                   ` (3 preceding siblings ...)
  2001-08-16 22:28 ` 2.4.9 does not compile [better PATCH] Anton Altaparmakov
@ 2001-08-16 22:31 ` David S. Miller
  2001-08-16 23:11   ` David S. Miller
                     ` (3 more replies)
  2001-08-16 23:22 ` Anton Altaparmakov
  2001-08-17  1:06 ` 2.4.9 does not compile (adaptec new driver) Luigi Genoni
  6 siblings, 4 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-16 22:31 UTC (permalink / raw)
  To: tpepper; +Cc: f5ibh, linux-kernel

   From: tpepper@vato.org
   Date: Thu, 16 Aug 2001 14:41:09 -0700

   Confirmed here.  Looks like a pretty obvious goof to me.  Does the following
   fix it for you?

The args and semantics of min/max changed to take
a type first argument, the problem with this ntfs file is that it
fails to include linux/kernel.h

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
@ 2001-08-16 23:11   ` David S. Miller
  2001-08-16 23:13   ` Daniel Phillips
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-16 23:11 UTC (permalink / raw)
  To: phillips; +Cc: tpepper, f5ibh, linux-kernel

   From: Daniel Phillips <phillips@bonn-fries.net>
   Date: Fri, 17 Aug 2001 01:13:38 +0200

   What is wrong with using typeof?

Users don't think about the type that way, the new macros forces them
to at least consider it for a moment.

   If you must have a three argument min, 
   could it please be called "type_min" of similar.

We wanted people trying to use "min" and "max" with 2
args, or redefining their own, to get a compile error.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
  2001-08-16 23:11   ` David S. Miller
@ 2001-08-16 23:13   ` Daniel Phillips
  2001-08-16 23:14     ` Alan Cox
  2001-08-16 23:35     ` Anton Altaparmakov
  2001-08-16 23:38   ` David S. Miller
  2001-08-17 17:30   ` Rik van Riel
  3 siblings, 2 replies; 56+ messages in thread
From: Daniel Phillips @ 2001-08-16 23:13 UTC (permalink / raw)
  To: David S. Miller, tpepper; +Cc: f5ibh, linux-kernel

On August 17, 2001 12:31 am, David S. Miller wrote:
>    From: tpepper@vato.org
>    Date: Thu, 16 Aug 2001 14:41:09 -0700
> 
>    Confirmed here.  Looks like a pretty obvious goof to me.  Does the 
following
>    fix it for you?
> 
> The args and semantics of min/max changed to take
> a type first argument,

They did?  This three argument min is butt-ugly, not to mention a completely 
original way of expressing the idea that is very much in conflict with every 
other expression of min I have ever seen.

What is wrong with using typeof?  If you must have a three argument min, 
could it please be called "type_min" of similar.

> the problem with this ntfs file is that it fails to include linux/kernel.h

--
Daniel

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:13   ` Daniel Phillips
@ 2001-08-16 23:14     ` Alan Cox
  2001-08-16 23:40       ` David S. Miller
  2001-08-16 23:35     ` Anton Altaparmakov
  1 sibling, 1 reply; 56+ messages in thread
From: Alan Cox @ 2001-08-16 23:14 UTC (permalink / raw)
  To: Daniel Phillips; +Cc: David S. Miller, tpepper, f5ibh, linux-kernel

> > The args and semantics of min/max changed to take
> > a type first argument,
> 
> They did?  This three argument min is butt-ugly, not to mention a completely 
> original way of expressing the idea that is very much in conflict with every 
> other expression of min I have ever seen.
> 
> What is wrong with using typeof?  If you must have a three argument min, 
> could it please be called "type_min" of similar.

It also doesnt solve all the cases. Basically its just ensuring everyone
doing portable code has to go and change all their macros to MIN and MAX
instead. Or use if statements, which with all the subtle suprises of
type casting is actually often a far far better idea and probably should
be encouraged a lot more in the kernel

Alan

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 21:11 2.4.9 does not compile f5ibh
                   ` (4 preceding siblings ...)
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
@ 2001-08-16 23:22 ` Anton Altaparmakov
  2001-08-17  1:06 ` 2.4.9 does not compile (adaptec new driver) Luigi Genoni
  6 siblings, 0 replies; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-16 23:22 UTC (permalink / raw)
  To: David S. Miller; +Cc: tpepper, f5ibh, linux-kernel

At 23:31 16/08/2001, David S. Miller wrote:
>    From: tpepper@vato.org
>    Date: Thu, 16 Aug 2001 14:41:09 -0700
>
>    Confirmed here.  Looks like a pretty obvious goof to me.  Does the 
> following
>    fix it for you?
>
>The args and semantics of min/max changed to take
>a type first argument, the problem with this ntfs file is that it
>fails to include linux/kernel.h

It has indeed. I do fail to see why that was necessary though...

IMHO, it would have been more elegant to use the typeof construct provided 
by gcc in the new macro instead of introducing a type parameter like this...

#define min(x,y) \
         ({ typeof(x) __x = (x); typeof(y) __y = (y); __x < __y ? __x: __y; })

Best regards,

         Anton


-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:13   ` Daniel Phillips
  2001-08-16 23:14     ` Alan Cox
@ 2001-08-16 23:35     ` Anton Altaparmakov
  2001-08-17  1:46       ` Jeff Dike
  1 sibling, 1 reply; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-16 23:35 UTC (permalink / raw)
  To: Alan Cox; +Cc: Daniel Phillips, David S. Miller, tpepper, f5ibh, linux-kernel

At 00:14 17/08/2001, Alan Cox wrote:
> > > The args and semantics of min/max changed to take
> > > a type first argument,
> >
> > They did?  This three argument min is butt-ugly, not to mention a 
> completely
> > original way of expressing the idea that is very much in conflict with 
> every
> > other expression of min I have ever seen.
> >
> > What is wrong with using typeof?  If you must have a three argument min,
> > could it please be called "type_min" of similar.
>
>It also doesnt solve all the cases.

Really? Could you point out an example where using ... typeof(x) __x; 
typeof(y) __y; ... in the macros wouldn't work? - I just tried a few 
examples I thought wouldn't work (side-effects ones) but I was pleasantly 
surprised to that gcc always produced the exact same assembler output for 
both the 3 arg and the 2 arg + typeof macros.

>Basically its just ensuring everyone doing portable code has to go and 
>change all their macros to MIN and MAX instead.

Considering the patch removed all occurences of MIN/MAX with the new 3 arg 
min/max macros it wouldn't seem that this would be accepted... )-: 
Otherwise I would submit a patch to switch NTFS. I don't like this 3 arg 
construct...

Best regards,

         Anton

>Or use if statements, which with all the subtle suprises of type casting 
>is actually often a far far better idea and probably should be encouraged 
>a lot more in the kernel



-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
  2001-08-16 23:11   ` David S. Miller
  2001-08-16 23:13   ` Daniel Phillips
@ 2001-08-16 23:38   ` David S. Miller
  2001-08-17  1:24     ` Roman Zippel
  2001-08-17  1:50     ` David S. Miller
  2001-08-17 17:30   ` Rik van Riel
  3 siblings, 2 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-16 23:38 UTC (permalink / raw)
  To: aia21; +Cc: tpepper, f5ibh, linux-kernel

   From: Anton Altaparmakov <aia21@cam.ac.uk>
   Date: Fri, 17 Aug 2001 00:22:43 +0100
   
   IMHO, it would have been more elegant to use the typeof construct provided 
   by gcc in the new macro instead of introducing a type parameter like this...

The whole point was to make users explicitly state the type so they
would have to think about it.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:14     ` Alan Cox
@ 2001-08-16 23:40       ` David S. Miller
  2001-08-17 21:58         ` David S. Miller
  0 siblings, 1 reply; 56+ messages in thread
From: David S. Miller @ 2001-08-16 23:40 UTC (permalink / raw)
  To: aia21; +Cc: alan, phillips, tpepper, f5ibh, linux-kernel

   From: Anton Altaparmakov <aia21@cam.ac.uk>
   Date: Fri, 17 Aug 2001 00:35:02 +0100

   Otherwise I would submit a patch to switch NTFS. I don't like this 3 arg 
   construct...

Linus will reject every such patch, and I have grepping scripts that
will detect crap like this entering the tree in case he misses
something.

Later,
David S. Miller
davem@redhat.com

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

* 2.4.9 does not compile (adaptec new driver)
  2001-08-16 21:11 2.4.9 does not compile f5ibh
                   ` (5 preceding siblings ...)
  2001-08-16 23:22 ` Anton Altaparmakov
@ 2001-08-17  1:06 ` Luigi Genoni
  6 siblings, 0 replies; 56+ messages in thread
From: Luigi Genoni @ 2001-08-17  1:06 UTC (permalink / raw)
  To: linux-kernel


HI, fast to patch, but still noisy

make[5]: Entering directory
`/usr/src/linux-2.4.9/drivers/scsi/aic7xxx/aicasm'
gcc -I/usr/include -I. -ldb aicasm_gram.c aicasm_scan.c aicasm.c
aicasm_symbol.c -o aicasm
aicasm/aicasm_scan.l: In function `yylex':
aicasm/aicasm_scan.l:115: `T_VERSION' undeclared (first use in this
function)
aicasm/aicasm_scan.l:115: (Each undeclared identifier is reported only
once
aicasm/aicasm_scan.l:115: for each function it appears in.)
aicasm/aicasm_scan.l:132: `T_STRING' undeclared (first use in this
function)
make[5]: *** [aicasm] Error 1
make[5]: Leaving directory
`/usr/src/linux-2.4.9/drivers/scsi/aic7xxx/aicasm'
make[4]: *** [aicasm/aicasm] Error 2
make[4]: Leaving directory `/usr/src/linux-2.4.9/drivers/scsi/aic7xxx'
make[3]: *** [first_rule] Error 2
make[3]: Leaving directory `/usr/src/linux-2.4.9/drivers/scsi/aic7xxx'
make[2]: *** [_subdir_aic7xxx] Error 2
make[2]: Leaving directory `/usr/src/linux-2.4.9/drivers/scsi'
make[1]: *** [_subdir_scsi] Error 2
make[1]: Leaving directory `/usr/src/linux-2.4.9/drivers'
make: *** [_dir_drivers] Error 2

bests
Luigi



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

* Re: 2.4.9 does not compile [PATCH]
       [not found]         ` <Your message of "Fri, 17 Aug 2001 00:35:02 +0100." <5.1.0.14.2.20010817002825.00b1e4e0@pop.cus.cam.ac.uk>
@ 2001-08-17  1:22           ` Anton Altaparmakov
  2001-08-17  1:49             ` Andrew Morton
                               ` (2 more replies)
  0 siblings, 3 replies; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-17  1:22 UTC (permalink / raw)
  To: Jeff Dike
  Cc: Alan Cox, Daniel Phillips, David S. Miller, tpepper, f5ibh, linux-kernel

Hi,

Thanks for the reply, but...

At 02:46 17/08/2001, Jeff Dike wrote:
>aia21@cam.ac.uk said:
> > Really? Could you point out an example where using ... typeof(x) __x;
> > typeof(y) __y; ... in the macros wouldn't work? - I just tried a few
> > examples I thought wouldn't work (side-effects ones) but I was
> > pleasantly  surprised to that gcc always produced the exact same
> > assembler output for  both the 3 arg and the 2 arg + typeof macros.
>
>Try min(a, min(b, c)).  Look at the cpp expansion and notice all the variable
>name clashes.

There are no clashes; unless my knowledge of C has abandoned me at the 
moment, you can reuse variable names within local statement blocks and the 
compiler is intelligent enough to sort it out.

I tried egcs-1.1.2/gcc-2.91.66, gcc 2.95.3 (SuSE 7.1) and gcc 2.96 (RHL 7.1 
2.96-85) and the generated assembler is fully correct with all these 
compilers for follwing little test (I compiled it with the same gcc command 
line as the kernel uses for the ntfs source files):

--- cut ---
#define min(x,y) \
         ({ typeof(x) __x = (x); typeof(y) __y = (y); __x < __y ? __x : __y; })

int test(int a, int b, int c)
{
         return min(a, min(b, c));
}
--- cut ---

No warnings are emitted and the assembler is correct if I am not mistaken.

Is my test invalid? I guess I must be missing something, but I can't figure 
out what. )-: Perhaps sleeping over it might help...

>We went through this on #kernel one night, and Alan concocted some amazingly
>gross unique identifier generators as a result.  To me, this looks like the
>best way to do this.

Shame I missed that... </me really ignorant> which IRC network is this 
channel on? Doesn't seem to be on openprojects.net...

Best regards,

Anton


-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:38   ` David S. Miller
@ 2001-08-17  1:24     ` Roman Zippel
  2001-08-17  1:50     ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  1:24 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

>    IMHO, it would have been more elegant to use the typeof construct provided
>    by gcc in the new macro instead of introducing a type parameter like this...
> 
> The whole point was to make users explicitly state the type so they
> would have to think about it.

I have two problems with this:
1. They maybe think once about it, but are they doing it a second time?
If the type of the argument is changed somewhere in the header, the min
argument is easily missed, since...
2. This macro doesn't produce a warning like the typeof version does.
The typeof version warns you about signed/unsigned compares, while an
assignment gives no warning.

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:35     ` Anton Altaparmakov
@ 2001-08-17  1:46       ` Jeff Dike
       [not found]         ` <Your message of "Fri, 17 Aug 2001 00:35:02 +0100." <5.1.0.14.2.20010817002825.00b1e4e0@pop.cus.cam.ac.uk>
  0 siblings, 1 reply; 56+ messages in thread
From: Jeff Dike @ 2001-08-17  1:46 UTC (permalink / raw)
  To: Anton Altaparmakov
  Cc: Alan Cox, Daniel Phillips, David S. Miller, tpepper, f5ibh, linux-kernel

aia21@cam.ac.uk said:
> Really? Could you point out an example where using ... typeof(x) __x;
> typeof(y) __y; ... in the macros wouldn't work? - I just tried a few
> examples I thought wouldn't work (side-effects ones) but I was
> pleasantly  surprised to that gcc always produced the exact same
> assembler output for  both the 3 arg and the 2 arg + typeof macros. 

Try min(a, min(b, c)).  Look at the cpp expansion and notice all the variable
name clashes.

We went through this on #kernel one night, and Alan concocted some amazingly
gross unique identifier generators as a result.  To me, this looks like the 
best way to do this.

				Jeff


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:22           ` Anton Altaparmakov
@ 2001-08-17  1:49             ` Andrew Morton
  2001-08-17  2:05               ` Daniel Phillips
  2001-08-17  1:53             ` David S. Miller
  2001-08-17  3:55             ` Jeff Dike
  2 siblings, 1 reply; 56+ messages in thread
From: Andrew Morton @ 2001-08-17  1:49 UTC (permalink / raw)
  To: Anton Altaparmakov
  Cc: Jeff Dike, Alan Cox, Daniel Phillips, David S. Miller, tpepper,
	f5ibh, linux-kernel

Anton Altaparmakov wrote:
> 
> #define min(x,y) \
>          ({ typeof(x) __x = (x); typeof(y) __y = (y); __x < __y ? __x : __y; })
> 
> int test(int a, int b, int c)
> {
>          return min(a, min(b, c));
> }

Problems occur if the caller happens to pass in variables whose
names confilct with the ones you chose in the above macro:

laptop:/home/akpm> cat t.c
#define min(x,y) \
         ({ typeof(x) __x = (x); typeof(y) __y = (y); __x < __y ? __x : __y; })

int test(int __x, int __y)
{
        return min(__x, __y);		/* sic */
}

main()
{
        printf("%d\n", test(1, 2));
        printf("%d\n", test(2, 1));
}
laptop:/home/akpm> gcc t.c
laptop:/home/akpm> ./a.out
-1073744344
1074305684


So if you replace "__x" with "__dont_use_this_identifier_its_reserved_for_min"
I don't expect anyone could sensibly complain...

-

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:38   ` David S. Miller
  2001-08-17  1:24     ` Roman Zippel
@ 2001-08-17  1:50     ` David S. Miller
  2001-08-17  2:29       ` Roman Zippel
  2001-08-17  2:38       ` David S. Miller
  1 sibling, 2 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  1:50 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 03:24:05 +0200

   2. This macro doesn't produce a warning like the typeof version does.
   The typeof version warns you about signed/unsigned compares, while an
   assignment gives no warning.

That is a legitimate operation, there is no reason to prevent people
from comparing signed and unsigned values.  These type argument
min/max values allow people to specify what the comparison type
degenerates into.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:22           ` Anton Altaparmakov
  2001-08-17  1:49             ` Andrew Morton
@ 2001-08-17  1:53             ` David S. Miller
  2001-08-17  3:55             ` Jeff Dike
  2 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  1:53 UTC (permalink / raw)
  To: akpm; +Cc: aia21, jdike, alan, phillips, tpepper, f5ibh, linux-kernel

   From: Andrew Morton <akpm@zip.com.au>
   Date: Thu, 16 Aug 2001 18:49:58 -0700

   int test(int __x, int __y)
   {
           return min(__x, __y);		/* sic */
   }

People are expected not to use underscore prefixed
variables in normal C code, this is why macros
in the kernel make liberal use of them for locals.

I mean, with your rule half of the macro defines in the kernel using
local variables are broken :-)

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:49             ` Andrew Morton
@ 2001-08-17  2:05               ` Daniel Phillips
  0 siblings, 0 replies; 56+ messages in thread
From: Daniel Phillips @ 2001-08-17  2:05 UTC (permalink / raw)
  To: Andrew Morton, Anton Altaparmakov
  Cc: Jeff Dike, Alan Cox, David S. Miller, tpepper, f5ibh, linux-kernel

On August 17, 2001 03:49 am, Andrew Morton wrote:
> Anton Altaparmakov wrote:
> > 
> > #define min(x,y) \
> >          ({ typeof(x) __x = (x); typeof(y) __y = (y); __x < __y ? __x : 
__y; })
> > 
> > int test(int a, int b, int c)
> > {
> >          return min(a, min(b, c));
> > }
> 
> Problems occur if the caller happens to pass in variables whose
> names confilct with the ones you chose in the above macro:

I remember the thread in detail.  We must have similar scoping problems in 
innumerable macros.  I'd a braindead flaw in C's scoping rules, and you know, 
I don't think it matters.  Don't use local variables with names like __foo.

--
Daniel

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:50     ` David S. Miller
@ 2001-08-17  2:29       ` Roman Zippel
  2001-08-17  2:38       ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  2:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

> That is a legitimate operation, there is no reason to prevent people
> from comparing signed and unsigned values.  These type argument
> min/max values allow people to specify what the comparison type
> degenerates into.

Sure, but if people change something, they get a warning and can think
_again_, your current macro prevents this. That hardcoded type is just
to easy to miss and can introduce subtle bugs. The typeof version
automatically chooses that largest of the types and possibly warns you.
Why would you want something different?

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:50     ` David S. Miller
  2001-08-17  2:29       ` Roman Zippel
@ 2001-08-17  2:38       ` David S. Miller
  2001-08-17  2:53         ` Roman Zippel
                           ` (3 more replies)
  1 sibling, 4 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  2:38 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 04:29:42 +0200
   
   Why would you want something different?

Because comparing a signed value with an unsigned value is a perfectly
sane operation and one should not have to put dumb casts into the
expression or change the types just to avoid a compiler warning.

That's dumb.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:38       ` David S. Miller
@ 2001-08-17  2:53         ` Roman Zippel
  2001-08-17  2:59         ` David S. Miller
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  2:53 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

> Because comparing a signed value with an unsigned value is a perfectly
> sane operation and one should not have to put dumb casts into the
> expression or change the types just to avoid a compiler warning.

Most of the time that cast isn't needed and rather an indicator that
something else is wrong, right now we don't even have a chance to detect
such a situation.
Second, what's wrong with letting the compiler choose the appropriate
type? The compiler will still do it, if you change one of the types, but
the hardcoded type in min is too easy miss.

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:38       ` David S. Miller
  2001-08-17  2:53         ` Roman Zippel
@ 2001-08-17  2:59         ` David S. Miller
  2001-08-17  3:23           ` Roman Zippel
                             ` (3 more replies)
  2001-08-17  3:08         ` Roman Zippel
  2001-08-17  3:13         ` David S. Miller
  3 siblings, 4 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  2:59 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 04:53:18 +0200

   Most of the time that cast isn't needed and rather an indicator that
   something else is wrong, right now we don't even have a chance to detect
   such a situation.

Wrong.  This is legal:

int test(unsigned long a, int b)
{
	return min(a, b);
}

And the compiler will warn about it with your typeof version.
That is dumb and unacceptable.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:38       ` David S. Miller
  2001-08-17  2:53         ` Roman Zippel
  2001-08-17  2:59         ` David S. Miller
@ 2001-08-17  3:08         ` Roman Zippel
  2001-08-17  3:13         ` David S. Miller
  3 siblings, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  3:08 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

> Because comparing a signed value with an unsigned value is a perfectly
> sane operation and one should not have to put dumb casts into the
> expression or change the types just to avoid a compiler warning.

BTW I just looked through the patch and I only found a single cast, so
there was not much need for such dumb casts. Your patch now forced the
cast into all of them...

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:38       ` David S. Miller
                           ` (2 preceding siblings ...)
  2001-08-17  3:08         ` Roman Zippel
@ 2001-08-17  3:13         ` David S. Miller
  2001-08-17  3:39           ` Roman Zippel
  2001-08-17  3:44           ` David S. Miller
  3 siblings, 2 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  3:13 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 05:08:40 +0200
   
   BTW I just looked through the patch and I only found a single cast, so
   there was not much need for such dumb casts. Your patch now forced the
   cast into all of them...

The cast in the new version is not dumb, it's smart.

It's the programmer saying (to both the reader of the
code and the compiler) "I want this comparison to use
type X".  Period.

There is no ambiguity, there are no multiple-evaluation
issues, and no dumb warnings from the compiler.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:59         ` David S. Miller
@ 2001-08-17  3:23           ` Roman Zippel
  2001-08-17  3:37           ` David S. Miller
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  3:23 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

> Wrong.  This is legal:
> 
> int test(unsigned long a, int b)
> {
>         return min(a, b);
> }
> 
> And the compiler will warn about it with your typeof version.
> That is dumb and unacceptable.

Please show me a place in the kernel where such code is used and is not
dumb.

Could you please answer my argument about maintainability?

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:59         ` David S. Miller
  2001-08-17  3:23           ` Roman Zippel
@ 2001-08-17  3:37           ` David S. Miller
  2001-08-17  3:59             ` Roman Zippel
  2001-08-17  8:35             ` Anton Altaparmakov
  2001-08-17  4:29           ` Ben LaHaise
  2001-08-17  6:07           ` David S. Miller
  3 siblings, 2 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  3:37 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 05:23:01 +0200

   Please show me a place in the kernel where such code is used and is
   not dumb.

Why don't you point out an example yourself?  You seem pretty
confident that a comparsion between a signed and unsigned value cannot
possible be legitimate.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  3:13         ` David S. Miller
@ 2001-08-17  3:39           ` Roman Zippel
  2001-08-17  3:44           ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  3:39 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

> The cast in the new version is not dumb, it's smart.

Yes, it's the smart version of "min((type)a, (type)b)"...

> It's the programmer saying (to both the reader of the
> code and the compiler) "I want this comparison to use
> type X".  Period.

Why? Why would anyone want this? Do you need this for any other ordinary
compare? (For which the compiler will also generate "dumb" warnings.)

> There is no ambiguity, there are no multiple-evaluation
> issues, and no dumb warnings from the compiler.

I'm all for fixing the multiple-evaluation, but I don't see why this
warning should be dumb. If you don't like it, use "-Wno-sign-compare".

bye, Roman

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  3:13         ` David S. Miller
  2001-08-17  3:39           ` Roman Zippel
@ 2001-08-17  3:44           ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  3:44 UTC (permalink / raw)
  To: zippel; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Roman Zippel <zippel@linux-m68k.org>
   Date: Fri, 17 Aug 2001 05:39:13 +0200

   Why? Why would anyone want this?

Ask Linus when he comes back from Finland. :-)

There are almost certainly threads deep in the linux-kernel archives
where Linus and others talk about this very issue.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  1:22           ` Anton Altaparmakov
  2001-08-17  1:49             ` Andrew Morton
  2001-08-17  1:53             ` David S. Miller
@ 2001-08-17  3:55             ` Jeff Dike
  2 siblings, 0 replies; 56+ messages in thread
From: Jeff Dike @ 2001-08-17  3:55 UTC (permalink / raw)
  To: Anton Altaparmakov
  Cc: Alan Cox, Daniel Phillips, David S. Miller, tpepper, f5ibh, linux-kernel

aia21@cam.ac.uk said:
> There are no clashes; unless my knowledge of C has abandoned me at the
>  moment, you can reuse variable names within local statement blocks
> and the  compiler is intelligent enough to sort it out. 

Sorry, you're right.  I wasn't thinking this through enough.  I was thinking 
that
	min(a, min(b, c))
would cause cpp to do things like
	int __x = (__x)

It seems the only problematic case is calling it with __x type arguments.

				Jeff


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  3:37           ` David S. Miller
@ 2001-08-17  3:59             ` Roman Zippel
  2001-08-17  8:35             ` Anton Altaparmakov
  1 sibling, 0 replies; 56+ messages in thread
From: Roman Zippel @ 2001-08-17  3:59 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

Hi,

"David S. Miller" wrote:

>    Please show me a place in the kernel where such code is used and is
>    not dumb.
> 
> Why don't you point out an example yourself?  You seem pretty
> confident that a comparsion between a signed and unsigned value cannot
> possible be legitimate.

No, I did not say this. I said that such warning would be useful and
that the majority of users does not need the forced cast. This forced
cast is IMO more dangerous than useful.
I'd prefer people think about the warning, than suppress that warning
and let people run into subtle bugs. For the majority of uses, this will
be a bug or at least carelessness. For the minority of legal uses add a
cast + comment and everything is fine.

bye, Roman

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

* Re: 2.4.9 does not compile [better PATCH]
  2001-08-16 22:28 ` 2.4.9 does not compile [better PATCH] Anton Altaparmakov
@ 2001-08-17  4:23   ` Jeff Chua
  0 siblings, 0 replies; 56+ messages in thread
From: Jeff Chua @ 2001-08-17  4:23 UTC (permalink / raw)
  To: Anton Altaparmakov; +Cc: tpepper, f5ibh, Linux Kernel


lvm does not compile either.

Here's the complete patch for 2.4.9

Jeff

--- linux/drivers/md/lvm.c.org	Fri Aug 17 12:07:35 2001
+++ linux/drivers/md/lvm.c	Fri Aug 17 12:09:05 2001
@@ -2326,7 +2326,7 @@

 	/* save availiable i/o statistic data */
 	if (old_lv->lv_stripes < 2) {	/* linear logical volume */
-		end = min(old_lv->lv_current_le, new_lv->lv_current_le);
+		end = min(__u32, old_lv->lv_current_le, new_lv->lv_current_le);
 		for (l = 0; l < end; l++) {
 			new_lv->lv_current_pe[l].reads +=
 				old_lv->lv_current_pe[l].reads;
@@ -2340,7 +2340,7 @@

 		old_stripe_size = old_lv->lv_allocated_le / old_lv->lv_stripes;
 		new_stripe_size = new_lv->lv_allocated_le / new_lv->lv_stripes;
-		end = min(old_stripe_size, new_stripe_size);
+		end = min(__u32, old_stripe_size, new_stripe_size);

 		for (i = source = dest = 0; i < new_lv->lv_stripes; i++) {
 			for (j = 0; j < end; j++) {
--- linux/drivers/md/lvm-snap.c.org	Fri Aug 17 12:09:35 2001
+++ linux/drivers/md/lvm-snap.c	Fri Aug 17 12:10:30 2001
@@ -360,8 +360,8 @@

 	blksize_org = lvm_get_blksize(org_phys_dev);
 	blksize_snap = lvm_get_blksize(snap_phys_dev);
-	max_blksize = max(blksize_org, blksize_snap);
-	min_blksize = min(blksize_org, blksize_snap);
+	max_blksize = max(__u32, blksize_org, blksize_snap);
+	min_blksize = min(__u32, blksize_org, blksize_snap);
 	max_sectors = KIO_MAX_SECTORS * (min_blksize>>9);

 	if (chunk_size % (max_blksize>>9))
@@ -369,7 +369,7 @@

 	while (chunk_size)
 	{
-		nr_sectors = min(chunk_size, max_sectors);
+		nr_sectors = min(__u32, chunk_size, max_sectors);
 		chunk_size -= nr_sectors;

 		iobuf->length = nr_sectors << 9;
@@ -486,7 +486,7 @@

 	buckets = lv->lv_remap_end;
 	max_buckets = calc_max_buckets();
-	buckets = min(buckets, max_buckets);
+	buckets = min(__u32, buckets, max_buckets);
 	while (buckets & (buckets-1))
 		buckets &= (buckets-1);

--- linux/fs/ntfs/unistr.c.org	Fri Aug 17 12:18:13 2001
+++ linux/fs/ntfs/unistr.c	Fri Aug 17 12:18:25 2001
@@ -23,6 +23,7 @@

 #include <linux/string.h>
 #include <asm/byteorder.h>
+#include <linux/kernel.h>

 #include "unistr.h"
 #include "macros.h"
@@ -96,7 +97,7 @@
 	__u32 cnt;
 	wchar_t c1, c2;

-	for (cnt = 0; cnt < min(unsigned int, name1_len, name2_len); ++cnt)
+	for (cnt = 0; cnt < min(__u32, name1_len, name2_len); ++cnt)
 	{
 		c1 = le16_to_cpu(*name1++);
 		c2 = le16_to_cpu(*name2++);


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:59         ` David S. Miller
  2001-08-17  3:23           ` Roman Zippel
  2001-08-17  3:37           ` David S. Miller
@ 2001-08-17  4:29           ` Ben LaHaise
  2001-08-17  6:07           ` David S. Miller
  3 siblings, 0 replies; 56+ messages in thread
From: Ben LaHaise @ 2001-08-17  4:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: zippel, aia21, tpepper, f5ibh, linux-kernel

On Thu, 16 Aug 2001, David S. Miller wrote:

> Wrong.  This is legal:
>
> int test(unsigned long a, int b)
> {
> 	return min(a, b);
> }
>
> And the compiler will warn about it with your typeof version.
> That is dumb and unacceptable.

I would hope that it would warn: what if a is the maximum size that an
array can be and b is a value passed in from userland?  Most definately
not an expected result.

		-ben


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  2:59         ` David S. Miller
                             ` (2 preceding siblings ...)
  2001-08-17  4:29           ` Ben LaHaise
@ 2001-08-17  6:07           ` David S. Miller
  3 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17  6:07 UTC (permalink / raw)
  To: bcrl; +Cc: zippel, aia21, tpepper, f5ibh, linux-kernel

   From: Ben LaHaise <bcrl@redhat.com>
   Date: Fri, 17 Aug 2001 00:29:58 -0400 (EDT)

   On Thu, 16 Aug 2001, David S. Miller wrote:
   
   > Wrong.  This is legal:
   >
   > int test(unsigned long a, int b)
   > {
   > 	return min(a, b);
   > }
   
   I would hope that it would warn: what if a is the maximum size that an
   array can be and b is a value passed in from userland?  Most definately
   not an expected result.

My example was poor, consider if 'b' was something like '100'
or for some other reason you already know perfectly well
the legal range of 'b' or 'a'.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17  3:37           ` David S. Miller
  2001-08-17  3:59             ` Roman Zippel
@ 2001-08-17  8:35             ` Anton Altaparmakov
  1 sibling, 0 replies; 56+ messages in thread
From: Anton Altaparmakov @ 2001-08-17  8:35 UTC (permalink / raw)
  To: David S. Miller; +Cc: zippel, aia21, tpepper, f5ibh, linux-kernel

On Thu, 16 Aug 2001, David S. Miller wrote:
>    From: Roman Zippel <zippel@linux-m68k.org>
>    Date: Fri, 17 Aug 2001 05:23:01 +0200
> 
>    Please show me a place in the kernel where such code is used and is
>    not dumb.
> 
> Why don't you point out an example yourself?  You seem pretty
> confident that a comparsion between a signed and unsigned value cannot
> possible be legitimate.

If you compare int x with unsigned int y you can get the wrong result in
the case that the unsigned value has the sign bit set and the signed
value is negative.

Example:

$ cat t.c
#include <stdio.h>

int main(void)
{
        int x = -2;
        unsigned int y = 0x80000000;

        printf("x = %i, y = %u\n", x, y);
        if (x < y)
                puts("x < y");
        else if (x > y)
                puts("x > y");
        else
                puts("x == y");
        return 0;
}
$ gcc -Wall t.c
$ ./a.out
x = -2, y = 2147483648
x > y

This is clearly wrong as -2 is less than 2147483648 and I doubt very much
that bug would be caught by introducing type casts into the min
function... It will just put people at ease that everything is now fine
when in fact it won't be (unless they make the type cast to long long
which I doubt people would do).

A bug simillar to this was present in NTFS a while ago as a matter of
fact and was causing massive corruption of run lists... And gcc doesn't
even emit a warning hence it took quite a lot of work to spot that this
was the bug. In the end I found it by looking at the generated corrupt
structures and realizing that there must be a sign bug somewhere and then
I looked at the function and I saw the light and fixed this properly by
using the same type of variables rather than playing silly games with
mixing signed and unsigned values.

If gcc had emitted a warning this bug would never have been possible to
occur so I am all for generating warnings, not suppressing them
artificially, at least in this case.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
                     ` (2 preceding siblings ...)
  2001-08-16 23:38   ` David S. Miller
@ 2001-08-17 17:30   ` Rik van Riel
  3 siblings, 0 replies; 56+ messages in thread
From: Rik van Riel @ 2001-08-17 17:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: tpepper, f5ibh, linux-kernel

On Thu, 16 Aug 2001, David S. Miller wrote:
>    From: tpepper@vato.org
>    Date: Thu, 16 Aug 2001 14:41:09 -0700
>
>    Confirmed here.  Looks like a pretty obvious goof to me.  Does the following
>    fix it for you?
>
> The args and semantics of min/max changed to take
> a type first argument, the problem with this ntfs file is that it
> fails to include linux/kernel.h

You should have checked this before deciding to rudely
bypass the maintainer of the file ;)

Rik
--
IA64: a worthy successor to the i860.

		http://www.surriel.com/
http://www.conectiva.com/	http://distro.conectiva.com/


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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:40       ` David S. Miller
@ 2001-08-17 21:58         ` David S. Miller
  0 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17 21:58 UTC (permalink / raw)
  To: riel; +Cc: aia21, alan, phillips, tpepper, f5ibh, linux-kernel

   From: Rik van Riel <riel@conectiva.com.br>
   Date: Fri, 17 Aug 2001 18:44:16 -0300 (BRST)

   On Thu, 16 Aug 2001, David S. Miller wrote:
   
   > Linus will reject every such patch, and I have grepping scripts
   > that will detect crap like this entering the tree in case he
   > misses something.
   
   Well, you're not the NTFS maintainer.

Yes, but Linus is the kernel maintainer.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17 22:11 ` David S. Miller
  2001-08-17 23:34   ` Daniel Phillips
@ 2001-08-17 23:38   ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17 23:38 UTC (permalink / raw)
  To: phillips; +Cc: alan, riel, tpepper, f5ibh, linux-kernel

   From: Daniel Phillips <phillips@bonn-fries.net>
   Date: Sat, 18 Aug 2001 01:34:28 +0200

   On August 18, 2001 12:11 am, David S. Miller wrote:
   > What bothers me is all the time being spent arguing about it.
   
   It would be safe to conclude it's a universally unpopular change.

Well, to be fair, popularity sometimes isn't much of an indicator.
Often people argue things because of change itself.  Having to do
something differently can make people feel uneasy, so they resist the
change.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-17 22:11 ` David S. Miller
@ 2001-08-17 23:34   ` Daniel Phillips
  2001-08-17 23:38   ` David S. Miller
  1 sibling, 0 replies; 56+ messages in thread
From: Daniel Phillips @ 2001-08-17 23:34 UTC (permalink / raw)
  To: David S. Miller, alan; +Cc: riel, tpepper, f5ibh, linux-kernel

On August 18, 2001 12:11 am, David S. Miller wrote:
> What bothers me is all the time being spent arguing about it.

It would be safe to conclude it's a universally unpopular change.

--
Daniel

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (10 preceding siblings ...)
  2001-08-17 22:09 ` Alan Cox
@ 2001-08-17 22:11 ` David S. Miller
  2001-08-17 23:34   ` Daniel Phillips
  2001-08-17 23:38   ` David S. Miller
  11 siblings, 2 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17 22:11 UTC (permalink / raw)
  To: alan; +Cc: riel, tpepper, f5ibh, linux-kernel

   From: Alan Cox <alan@lxorguk.ukuu.org.uk>
   Date: Fri, 17 Aug 2001 23:09:15 +0100 (BST)

   > If Linus scribbled all over the network code I would
   > have to accept it if it were a kernel wide cleanup he
   > absolutely wanted.
   
   That doesn't make it right, it merely makes the perpetrator wrong.

To be honest, I like having someone like him who can just put his foot
down on a matter whilst the rest of us waste days in silly flame wars
on the topic. :-)

I've stop caring, personally.  Back it out, it wouldn't bother me.
I made a patch someone asked me to make, so sue me.

What bothers me is all the time being spent arguing about it.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (9 preceding siblings ...)
  2001-08-17 21:40 ` Alan Cox
@ 2001-08-17 22:09 ` Alan Cox
  2001-08-17 22:11 ` David S. Miller
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-17 22:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: riel, alan, tpepper, f5ibh, linux-kernel

> If Linus scribbled all over the network code I would
> have to accept it if it were a kernel wide cleanup he
> absolutely wanted.

That doesn't make it right, it merely makes the perpetrator wrong.

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 23:02 ` David S. Miller
@ 2001-08-17 21:59   ` David S. Miller
  0 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17 21:59 UTC (permalink / raw)
  To: riel; +Cc: alan, tpepper, f5ibh, linux-kernel

   From: Rik van Riel <riel@conectiva.com.br>
   Date: Fri, 17 Aug 2001 18:46:20 -0300 (BRST)

   On Thu, 16 Aug 2001, David S. Miller wrote:
   
   > I don't recall you saying anything about "make sure to tell the
   > maintainers"
   
   I'll remember your attitude for if I ever have to
   touch the network code ;)

If Linus scribbled all over the network code I would
have to accept it if it were a kernel wide cleanup he
absolutely wanted.

Later,
David S. Miller
davem@redhat.com
   

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (8 preceding siblings ...)
  2001-08-17 20:57 ` David S. Miller
@ 2001-08-17 21:40 ` Alan Cox
  2001-08-17 22:09 ` Alan Cox
  2001-08-17 22:11 ` David S. Miller
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-17 21:40 UTC (permalink / raw)
  To: David S. Miller; +Cc: alan, aia21, tpepper, f5ibh, linux-kernel

>    Its actually basically impossible to do back compat macros
>    with this mess. Your original smin() umin() proposal was _much_ saner.
> 
> I don't see how you can logically say this.

Because it would have been trivial to provide back compatibility macros to
provide sint_min/sint_max on 2.2. Now all I can do is make -ac use sint_min
sint_max and friends (or typed_min to be exact) and make 2.2 and -ac 
compatible

> My sint_min() etc. version broke things just
> as equally because it had:

Nothing like as badly.

Alan

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

* Re: 2.4.9 does not compile [PATCH]
  2001-08-16 22:48 ` David S. Miller
@ 2001-08-17 21:12   ` Jes Sorensen
  0 siblings, 0 replies; 56+ messages in thread
From: Jes Sorensen @ 2001-08-17 21:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: alan, linux-kernel

>>>>> "David" == David S Miller <davem@redhat.com> writes:

David>    From: Alan Cox <alan@lxorguk.ukuu.org.uk> Date: Thu, 16 Aug
David> 2001 23:41:07 +0100 (BST)

>    Thank you for your detailed discussion of this in advance on
> the kernel list, your careful consideration of the 2.2
> compatibility work horrors you introduced and the thoughtful
> way you notified maintainers.

David> Listen:

David> 1) All of this was done at the request of Linus.

David> 2) You were CC:'d on every single email that Linus and I had on
David> these changes, and you saw every single revision of the patch.

Alan's point about CC'ing maintainers still stands, where was the CC
to the individual maintainers of the code you changed?

Jes

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (7 preceding siblings ...)
  2001-08-17  9:25 ` Alan Cox
@ 2001-08-17 20:57 ` David S. Miller
  2001-08-17 21:40 ` Alan Cox
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: David S. Miller @ 2001-08-17 20:57 UTC (permalink / raw)
  To: alan; +Cc: aia21, tpepper, f5ibh, linux-kernel

   From: Alan Cox <alan@lxorguk.ukuu.org.uk>
   Date: Fri, 17 Aug 2001 10:11:17 +0100 (BST)
   
   Its actually basically impossible to do back compat macros
   with this mess. Your original smin() umin() proposal was _much_ saner.

I don't see how you can logically say this.
My sint_min() etc. version broke things just
as equally because it had:

#define min __compile_error_do_not_use_min
#define max __compile_error_do_not_use_max

in it.  Do you think Linus or myself, by doing
this, intended to let anyone undef the damn things
to get around this?

The whole point of the changes was "min and max are
dumb, nobody may use them in their traditional form".

I was pretty sure, you understood this.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (6 preceding siblings ...)
  2001-08-17  9:17 ` Alan Cox
@ 2001-08-17  9:25 ` Alan Cox
  2001-08-17 20:57 ` David S. Miller
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-17  9:25 UTC (permalink / raw)
  To: David S. Miller; +Cc: bcrl, zippel, aia21, tpepper, f5ibh, linux-kernel

>    I would hope that it would warn: what if a is the maximum size that an
>    array can be and b is a value passed in from userland?  Most definately
>    not an expected result.
> 
> My example was poor, consider if 'b' was something like '100'
> or for some other reason you already know perfectly well
> the legal range of 'b' or 'a'.

Then the compiler will cast constants for you (and warn if they dont fit),
and you can use casts to make it clear you know the legal ranges

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (5 preceding siblings ...)
  2001-08-17  9:11 ` Alan Cox
@ 2001-08-17  9:17 ` Alan Cox
  2001-08-17  9:25 ` Alan Cox
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-17  9:17 UTC (permalink / raw)
  To: David S. Miller; +Cc: zippel, aia21, tpepper, f5ibh, linux-kernel

> Wrong.  This is legal:
> 
> int test(unsigned long a, int b)
> {
> 	return min(a, b);
> }

> And the compiler will warn about it with your typeof version.

Good, because its absolutely bogus and you want people to be warned so they
fix the input types to fit in the output type, and are forced to think about
whether all cases actually fit

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (4 preceding siblings ...)
  2001-08-16 23:08 ` Alan Cox
@ 2001-08-17  9:11 ` Alan Cox
  2001-08-17  9:17 ` Alan Cox
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-17  9:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: aia21, tpepper, f5ibh, linux-kernel

>    From: Anton Altaparmakov <aia21@cam.ac.uk>
>    Date: Fri, 17 Aug 2001 00:22:43 +0100
>    
>    IMHO, it would have been more elegant to use the typeof construct provided 
>    by gcc in the new macro instead of introducing a type parameter like this...
> 
> The whole point was to make users explicitly state the type so they
> would have to think about it.

And doing it by forcing them all to change their macro names isnt the
right solution. Its actually basically impossible to do back compat macros
with this mess. Your original smin() umin() proposal was _much_ saner.

As I've said, -ac will use typed_min(a,b,c), and that way I can propogate
back compatibility glue.

Alan

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

* Re: 2.4.9 does not compile [PATCH]
       [not found]     ` <20010816.185319.88475216.davem@redhat.com.suse.lists.linux.kernel>
@ 2001-08-17  7:30       ` Andi Kleen
  0 siblings, 0 replies; 56+ messages in thread
From: Andi Kleen @ 2001-08-17  7:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: linux-kernel

"David S. Miller" <davem@redhat.com> writes:

>    From: Andrew Morton <akpm@zip.com.au>
>    Date: Thu, 16 Aug 2001 18:49:58 -0700
> 
>    int test(int __x, int __y)
>    {
>            return min(__x, __y);		/* sic */
>    }
> 
> People are expected not to use underscore prefixed
> variables in normal C code, this is why macros
> in the kernel make liberal use of them for locals.

You are joking, right?  The kernel is full of double under score prefixed
identifiers, for functions that do slighter lower level things than others.
While this expectation may exist in POSIX/C89 and is frequently violated there,
in kernel C nobody cares about it at all.

It doesn't matter anyways, the way C macro expansion works guarantees that
only macro arguments written in the macro get expanded; the arguments are not
recursively expanded. Therefore any games with "magic" macro names 
is totally unnecessary.

-Andi

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (3 preceding siblings ...)
  2001-08-16 23:02 ` David S. Miller
@ 2001-08-16 23:08 ` Alan Cox
  2001-08-17  9:11 ` Alan Cox
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-16 23:08 UTC (permalink / raw)
  To: David S. Miller; +Cc: alan, tpepper, f5ibh, linux-kernel

> I don't recall you saying anything about "make sure to tell the
> maintainers" But, on the other hand, I can't prove that you didn't (I
> don't keep detailed mail logs anymore except for what I send out
> myself), so if you did I apologize.

I'm pretty sure I did (and I can prove it but whatever 8)). Whats done is done.
I think it was a bad idea for 2.4 (driver compat nightmare). 

Anyway arguing about it achieves nothing for anyone

Alan

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
                   ` (2 preceding siblings ...)
  2001-08-16 22:55 ` Alan Cox
@ 2001-08-16 23:02 ` David S. Miller
  2001-08-17 21:59   ` David S. Miller
  2001-08-16 23:08 ` Alan Cox
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 56+ messages in thread
From: David S. Miller @ 2001-08-16 23:02 UTC (permalink / raw)
  To: alan; +Cc: tpepper, f5ibh, linux-kernel

   From: Alan Cox <alan@lxorguk.ukuu.org.uk>
   Date: Thu, 16 Aug 2001 23:55:44 +0100 (BST)

   > 2) You were CC:'d on every single email
   >    that Linus and I had on these changes, and you
   >    saw every single revision of the patch.
   
   Yep. And at the time I asked you to send it to the maintainers and the like.

When you were shown the patch your sentiments were:

1) Some of that stuff is against old drivers, I have newer
   ones in -ac so no biggie.
2) The ixj instances look like real bugs, and that you'd
   have a closer look.
3) If there are any merge issues in your -ac tree, no big
   deal because they'd show up as obvious compile errors.
4) Otherwise, it looked just fine to you.

I don't recall you saying anything about "make sure to tell the
maintainers" But, on the other hand, I can't prove that you didn't (I
don't keep detailed mail logs anymore except for what I send out
myself), so if you did I apologize.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
  2001-08-16 22:41 ` 2.4.9 does not compile [PATCH] Alan Cox
  2001-08-16 22:48 ` David S. Miller
@ 2001-08-16 22:55 ` Alan Cox
  2001-08-16 23:02 ` David S. Miller
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-16 22:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: alan, tpepper, f5ibh, linux-kernel

> 2) You were CC:'d on every single email
>    that Linus and I had on these changes, and you
>    saw every single revision of the patch.

Yep. And at the time I asked you to send it to the maintainers and the like.

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
  2001-08-16 22:41 ` 2.4.9 does not compile [PATCH] Alan Cox
@ 2001-08-16 22:48 ` David S. Miller
  2001-08-17 21:12   ` Jes Sorensen
  2001-08-16 22:55 ` Alan Cox
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 56+ messages in thread
From: David S. Miller @ 2001-08-16 22:48 UTC (permalink / raw)
  To: alan; +Cc: tpepper, f5ibh, linux-kernel

   From: Alan Cox <alan@lxorguk.ukuu.org.uk>
   Date: Thu, 16 Aug 2001 23:41:07 +0100 (BST)

   > The args and semantics of min/max changed to take
   > a type first argument, the problem with this ntfs file is that it
   > fails to include linux/kernel.h
   
   Thank you for your detailed discussion of this in advance on the kernel
   list, your careful consideration of the 2.2 compatibility work horrors you
   introduced and the thoughtful way you notified maintainers.

Listen:

1) All of this was done at the request of Linus.

2) You were CC:'d on every single email
   that Linus and I had on these changes, and you
   saw every single revision of the patch.

Why are you complaining now, please speak up earlier next time when
you have a grip.  This is why you were CC:'d on the emails.

I am actually unhappy Linus waited until 2.4.9 final to put those
changes in as I was sure 1 or 2 compile issues would slip through.
I have been sending it over and over throughout the 2.4.9 prepatches.

Later,
David S. Miller
davem@redhat.com

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

* Re: 2.4.9 does not compile [PATCH]
       [not found] <no.id>
@ 2001-08-16 22:41 ` Alan Cox
  2001-08-16 22:48 ` David S. Miller
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Alan Cox @ 2001-08-16 22:41 UTC (permalink / raw)
  To: David S. Miller; +Cc: tpepper, f5ibh, linux-kernel

> The args and semantics of min/max changed to take
> a type first argument, the problem with this ntfs file is that it
> fails to include linux/kernel.h

Thank you for your detailed discussion of this in advance on the kernel
list, your careful consideration of the 2.2 compatibility work horrors you
introduced and the thoughtful way you notified maintainers.

And all this from a man who gives me an earful if I merge third party
network patches without going through him.

Alan

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

end of thread, other threads:[~2001-08-17 23:40 UTC | newest]

Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-08-16 21:11 2.4.9 does not compile f5ibh
2001-08-16 21:41 ` 2.4.9 does not compile [PATCH] tpepper
2001-08-16 22:10 ` 2.4.9 does not compile [better PATCH] tpepper
2001-08-16 22:21 ` 2.4.9 does not compile [PATCH] Anton Altaparmakov
2001-08-16 22:28 ` 2.4.9 does not compile [better PATCH] Anton Altaparmakov
2001-08-17  4:23   ` Jeff Chua
2001-08-16 22:31 ` 2.4.9 does not compile [PATCH] David S. Miller
2001-08-16 23:11   ` David S. Miller
2001-08-16 23:13   ` Daniel Phillips
2001-08-16 23:14     ` Alan Cox
2001-08-16 23:40       ` David S. Miller
2001-08-17 21:58         ` David S. Miller
2001-08-16 23:35     ` Anton Altaparmakov
2001-08-17  1:46       ` Jeff Dike
     [not found]         ` <Your message of "Fri, 17 Aug 2001 00:35:02 +0100." <5.1.0.14.2.20010817002825.00b1e4e0@pop.cus.cam.ac.uk>
2001-08-17  1:22           ` Anton Altaparmakov
2001-08-17  1:49             ` Andrew Morton
2001-08-17  2:05               ` Daniel Phillips
2001-08-17  1:53             ` David S. Miller
2001-08-17  3:55             ` Jeff Dike
2001-08-16 23:38   ` David S. Miller
2001-08-17  1:24     ` Roman Zippel
2001-08-17  1:50     ` David S. Miller
2001-08-17  2:29       ` Roman Zippel
2001-08-17  2:38       ` David S. Miller
2001-08-17  2:53         ` Roman Zippel
2001-08-17  2:59         ` David S. Miller
2001-08-17  3:23           ` Roman Zippel
2001-08-17  3:37           ` David S. Miller
2001-08-17  3:59             ` Roman Zippel
2001-08-17  8:35             ` Anton Altaparmakov
2001-08-17  4:29           ` Ben LaHaise
2001-08-17  6:07           ` David S. Miller
2001-08-17  3:08         ` Roman Zippel
2001-08-17  3:13         ` David S. Miller
2001-08-17  3:39           ` Roman Zippel
2001-08-17  3:44           ` David S. Miller
2001-08-17 17:30   ` Rik van Riel
2001-08-16 23:22 ` Anton Altaparmakov
2001-08-17  1:06 ` 2.4.9 does not compile (adaptec new driver) Luigi Genoni
     [not found] <no.id>
2001-08-16 22:41 ` 2.4.9 does not compile [PATCH] Alan Cox
2001-08-16 22:48 ` David S. Miller
2001-08-17 21:12   ` Jes Sorensen
2001-08-16 22:55 ` Alan Cox
2001-08-16 23:02 ` David S. Miller
2001-08-17 21:59   ` David S. Miller
2001-08-16 23:08 ` Alan Cox
2001-08-17  9:11 ` Alan Cox
2001-08-17  9:17 ` Alan Cox
2001-08-17  9:25 ` Alan Cox
2001-08-17 20:57 ` David S. Miller
2001-08-17 21:40 ` Alan Cox
2001-08-17 22:09 ` Alan Cox
2001-08-17 22:11 ` David S. Miller
2001-08-17 23:34   ` Daniel Phillips
2001-08-17 23:38   ` David S. Miller
     [not found] <Your message of "Fri, 17 Aug 2001 00:35:02 +0100."  <5.1.0.14.2.20010817002825.00b1e4e0@pop.cus.cam.ac.uk.suse.lists.linux.kernel>
     [not found] ` <5.1.0.14.2.20010817015007.045689b0@pop.cus.cam.ac.uk.suse.lists.linux.kernel>
     [not found]   ` <3B7C7846.FD9DEE68@zip.com.au.suse.lists.linux.kernel>
     [not found]     ` <20010816.185319.88475216.davem@redhat.com.suse.lists.linux.kernel>
2001-08-17  7:30       ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).