* 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 [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-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 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
* 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-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 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-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: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: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: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: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: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 [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-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 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 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-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 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
* 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] <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
* 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]
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>
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>
` (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]
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>
` (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>
` (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] <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>
` (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>
` (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>
` (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]
[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]
[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]
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]
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