This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
- arch/i386/pci/pcbios.c: use of "+m" constraint
- drivers/char/ftape/: use of cast-as-lvalue
- drivers/char/ftape/: __attribute__((packed)) on something
containing only bytes
Compiles cleanly and works for me.
This isn't critical, I'll resend after 2.6.6 final if
you don't want to merge it right now.
/Mikael
diff -ruN linux-2.6.6-rc3/arch/i386/pci/pcbios.c linux-2.6.6-rc3.gcc340-fixes/arch/i386/pci/pcbios.c
--- linux-2.6.6-rc3/arch/i386/pci/pcbios.c 2003-09-09 14:22:28.000000000 +0200
+++ linux-2.6.6-rc3.gcc340-fixes/arch/i386/pci/pcbios.c 2004-04-28 12:21:00.000000000 +0200
@@ -431,11 +431,13 @@
"1:"
: "=a" (ret),
"=b" (map),
- "+m" (opt)
+ "=m" (opt)
: "0" (PCIBIOS_GET_ROUTING_OPTIONS),
"1" (0),
"D" ((long) &opt),
- "S" (&pci_indirect));
+ "S" (&pci_indirect),
+ "m" (opt)
+ : "memory");
DBG("OK ret=%d, size=%d, map=%x\n", ret, opt.size, map);
if (ret & 0xff00)
printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
diff -ruN linux-2.6.6-rc3/drivers/char/ftape/lowlevel/ftape-bsm.c linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/lowlevel/ftape-bsm.c
--- linux-2.6.6-rc3/drivers/char/ftape/lowlevel/ftape-bsm.c 2002-02-20 03:10:58.000000000 +0100
+++ linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/lowlevel/ftape-bsm.c 2004-04-28 12:21:00.000000000 +0200
@@ -203,6 +203,7 @@
ft_format_code == fmt_1100ft) {
SectorCount *ptr = (SectorCount *)bad_sector_map;
unsigned int sector;
+ __u16 *ptr16;
while((sector = get_sector(ptr++)) != 0) {
if ((ft_format_code == fmt_big ||
@@ -218,9 +219,10 @@
}
/* Display old ftape's end-of-file marks
*/
- while ((sector = get_unaligned(((__u16*)ptr)++)) != 0) {
+ ptr16 = (__u16*)ptr;
+ while ((sector = get_unaligned(ptr16++)) != 0) {
TRACE(ft_t_noise, "Old ftape eof mark: %4d/%2d",
- sector, get_unaligned(((__u16*)ptr)++));
+ sector, get_unaligned(ptr16++));
}
} else { /* fixed size format */
for (i = ft_first_data_segment;
diff -ruN linux-2.6.6-rc3/drivers/char/ftape/lowlevel/ftape-bsm.h linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/lowlevel/ftape-bsm.h
--- linux-2.6.6-rc3/drivers/char/ftape/lowlevel/ftape-bsm.h 2002-02-20 03:10:52.000000000 +0100
+++ linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/lowlevel/ftape-bsm.h 2004-04-28 12:21:00.000000000 +0200
@@ -47,7 +47,7 @@
*/
typedef struct NewSectorMap {
__u8 bytes[3];
-} SectorCount __attribute__((packed));
+} SectorCount;
/*
diff -ruN linux-2.6.6-rc3/drivers/char/ftape/zftape/zftape-eof.c linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/zftape/zftape-eof.c
--- linux-2.6.6-rc3/drivers/char/ftape/zftape/zftape-eof.c 2003-02-24 23:25:37.000000000 +0100
+++ linux-2.6.6-rc3.gcc340-fixes/drivers/char/ftape/zftape/zftape-eof.c 2004-04-28 12:21:00.000000000 +0200
@@ -123,7 +123,7 @@
while (ptr + 3 < limit) {
if (get_unaligned((__u32*)ptr)) {
- ++(__u32*)ptr;
+ ptr += sizeof(__u32);
} else {
return ptr;
}
Mikael Pettersson wrote:
> This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
> - drivers/char/ftape/: use of cast-as-lvalue
> if (get_unaligned((__u32*)ptr)) {
> - ++(__u32*)ptr;
> + ptr += sizeof(__u32);
> } else {
Can anyone explain what is the problem with this?
To me it seems pretty ligitimate code - why it was outlawed in gcc 3.4?
Previous code was agnostic to type of ptr, but you code presume ptr
being char pointer (to effectively increment by 4 bytes).
So what all this buzz about?
On Apr 29, 2004, at 11:30, Ihar 'Philips' Filipau wrote:
> Mikael Pettersson wrote:
>> This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
>> - drivers/char/ftape/: use of cast-as-lvalue
>> if (get_unaligned((__u32*)ptr)) {
>> - ++(__u32*)ptr;
>> + ptr += sizeof(__u32);
>> } else {
>
> Can anyone explain what is the problem with this?
> To me it seems pretty ligitimate code - why it was outlawed in gcc
> 3.4?
http://es-sun2.fernuni-hagen.de/cgi-bin/info2html?(gcc)Subscripting
The ability to manipulate a cast was always a gcc extension, whether by
design or feature I am not sure. The problem is that it breaks C++
templates in a bad way. Here is where the extension is documented for
GCC 3.3.3
http://gcc.gnu.org/onlinedocs/gcc-3.3.3/gcc/Lvalues.html
This extension is now obsoleted.
> Previous code was agnostic to type of ptr, but you code presume ptr
> being char pointer (to effectively increment by 4 bytes).
Yes, and the previous code assumed that that ptr was not a pointer to a
64 bit value, since dereferencing it on most platforms would then
explode. I bet that both assumptions are just as safe ;-) However if
you really wanted to be safe then you could do
ptr += sizeof (__32) / sizeof (*ptr);
Again, assuming that we are not using a greater then 4 byte ptr*, since
otherwise it would not increment... But personally I think that this is
not required.
Cheers,
Paul
On Thursday 29 April 2004 12:30, Ihar 'Philips' Filipau wrote:
> Mikael Pettersson wrote:
> > This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
> > - drivers/char/ftape/: use of cast-as-lvalue
> > if (get_unaligned((__u32*)ptr)) {
> > - ++(__u32*)ptr;
> > + ptr += sizeof(__u32);
> > } else {
>
> Can anyone explain what is the problem with this?
> To me it seems pretty ligitimate code - why it was outlawed in gcc 3.4?
cast is not a lvalue. ++(__u32*)ptr is nonsense, just like ++4.
> Previous code was agnostic to type of ptr, but you code presume ptr
> being char pointer (to effectively increment by 4 bytes).
This would be agnostic too:
ptr = (void*) ((char*)ptr + sizeof(__u32));
--
vda
Denis Vlasenko wrote:
> On Thursday 29 April 2004 12:30, Ihar 'Philips' Filipau wrote:
>
>>Mikael Pettersson wrote:
>>
>>>This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
>>>- drivers/char/ftape/: use of cast-as-lvalue
>>> if (get_unaligned((__u32*)ptr)) {
>>>- ++(__u32*)ptr;
>>>+ ptr += sizeof(__u32);
>>> } else {
>>
>> Can anyone explain what is the problem with this?
>> To me it seems pretty ligitimate code - why it was outlawed in gcc 3.4?
>
>
> cast is not a lvalue. ++(__u32*)ptr is nonsense, just like ++4.
>
Yes. I see. C goes in direction of C++.
For me cast in C is always "forget original type and assume this type."
From this standpoint it makes a lot of sense. Is just like
((stuct { char a,b,c,d;})4).a - never tryed it in gcc (3.3 does not
accept it), but IIRC was possible on old PC C compilers - Turbo C 2/3 -
not sure name/version, but I used it. (Yes! recalled - M$VC/winbase.h
use this kind of macros for manipulating bytes/words/dwords/etc)
And in old C classes it was always said that in C you can convert
anything to everything, it just size of types must be the same.
Why not after all?
>
> ptr = (void*) ((char*)ptr + sizeof(__u32));
No Nice, But Accepted.
I have just checked my code base - I do not use this feature in any
way ;-)
--
Ihar 'Philips' Filipau / with best regards from Saarbruecken.
-- _ _ _
A programmer is a person who passes as an exacting expert |_|*|_|
on the basis of being able to turn out, after innumerable |_|_|*|
punching, an infinite series of incomprehensible answers |*|*|*|
calculated with micrometric precisions from vague
assumptions based on debatable figures taken from inconclusive
documents and carried out on instruments of problematical accuracy
by persons of dubious reliability and questionable mentality for
the avowed purpose of annoying and confounding a hopelessly
defenseless department that was unfortunate enough to ask for the
information in the first place.
-- IEEE Grid newsmagazine
On Thu, 29 Apr 2004 11:30:25 +0200, Ihar 'Philips' Filipau wrote:
>Mikael Pettersson wrote:
>> This patch fixes three warnings from gcc-3.4.0 in 2.6.6-rc3:
>> - drivers/char/ftape/: use of cast-as-lvalue
>> if (get_unaligned((__u32*)ptr)) {
>> - ++(__u32*)ptr;
>> + ptr += sizeof(__u32);
>> } else {
>
> Can anyone explain what is the problem with this?
> To me it seems pretty ligitimate code - why it was outlawed in gcc 3.4?
>
> Previous code was agnostic to type of ptr, but you code presume ptr
>being char pointer (to effectively increment by 4 bytes).
'ptr' _is_ a char pointer, and the code (visible in the part of
the patch you didn't include) already performed pointer arithmetic
on it relying on it being a char pointer. The old code had no
sane reason at all for updating 'ptr' via a cast-as-lvalue.
cast-as-lvalue is not proper C, has dodgey semantics, and can
always be replaced by proper C.
H. Peter Anvin writes:
> Followup to: <[email protected]>
> By author: Mikael Pettersson <[email protected]>
> In newsgroup: linux.dev.kernel
> >
> > 'ptr' _is_ a char pointer, and the code (visible in the part of
> > the patch you didn't include) already performed pointer arithmetic
> > on it relying on it being a char pointer. The old code had no
> > sane reason at all for updating 'ptr' via a cast-as-lvalue.
> >
> > cast-as-lvalue is not proper C, has dodgey semantics, and can
> > always be replaced by proper C.
> >
>
> I don't see how it has dodgey semantics for cast of pointers or
> [u]intptr_t to pointers.
You're assuming pointers have uniform representation.
C makes no such guarantees, and machines _have_ had
different types of representations in the past.
Some not-so-obsolete 64-bit machines in effect use fat
representations for pointers to functions (descriptors),
but they usually cheat and use pointers to the descriptors
instead. However, a C implementation could legally
represent a function pointer as a 128-bit value, while
data pointers remain 64 bits.
A cast fundamentally involves an assignment conversion,
a copy to a temporary, and it yields an rvalue.
Even if we allow its use as an lvalue, the semantics
would still be to assign the copy not the original.
So cast-as-lvalue as gcc implemented it changed two
major aspects of the semantics. Call me conservative
if you like, but that's simply not C any more.
Other gcc extensions, such as __inline__, __attribute__,
and __asm__, do provide useful and sensible features.
The issue with cast-as-lvalue is that it is neither
necessary nor does it promote maintainable and portable code.
Remember "the world's not a VAX" and "the world's not
a 68K with 24-bit addresses" lessons of the 80s,
> IMNSHO the fact that it breaks C++ isn't a good reason to outlaw a
> long-documented extension for C.
I couldn't care less about C++. There are ample reasons
why it's a bad idea in C itself.
/Mikael
On Tue, Jun 01, 2004 at 04:52:59PM +0200, Mikael Pettersson wrote:
> You're assuming pointers have uniform representation.
> C makes no such guarantees, and machines _have_ had
> different types of representations in the past.
> Some not-so-obsolete 64-bit machines in effect use fat
> representations for pointers to functions (descriptors),
> but they usually cheat and use pointers to the descriptors
> instead. However, a C implementation could legally
> represent a function pointer as a 128-bit value, while
> data pointers remain 64 bits.
IIRC for all types foo, sizeof(foo *) <= sizeof(void *), no?
If so, 128-bit function pointers implies >= 128-bit void pointers.
On Tue, Jun 01, 2004 at 04:52:59PM +0200, Mikael Pettersson wrote:
> A cast fundamentally involves an assignment conversion,
> a copy to a temporary, and it yields an rvalue.
> Even if we allow its use as an lvalue, the semantics
> would still be to assign the copy not the original.
> So cast-as-lvalue as gcc implemented it changed two
> major aspects of the semantics. Call me conservative
> if you like, but that's simply not C any more.
Oh, yeah, lvalue casting is degenerate filth.
-- wli
William Lee Irwin III writes:
> On Tue, Jun 01, 2004 at 04:52:59PM +0200, Mikael Pettersson wrote:
> > You're assuming pointers have uniform representation.
> > C makes no such guarantees, and machines _have_ had
> > different types of representations in the past.
> > Some not-so-obsolete 64-bit machines in effect use fat
> > representations for pointers to functions (descriptors),
> > but they usually cheat and use pointers to the descriptors
> > instead. However, a C implementation could legally
> > represent a function pointer as a 128-bit value, while
> > data pointers remain 64 bits.
>
> IIRC for all types foo, sizeof(foo *) <= sizeof(void *), no?
> If so, 128-bit function pointers implies >= 128-bit void pointers.
No, sizeof(foo*) <= sizeof(void*) only holds for data pointers.
The C standard is very explicit about not guaranteeing any
relationship between function pointers and void*. However,
a function pointer can be converted to a pointer to a different
function type and back again, without loss of information.
/Mikael
William Lee Irwin III <[email protected]> writes:
> On Tue, Jun 01, 2004 at 04:52:59PM +0200, Mikael Pettersson wrote:
>> You're assuming pointers have uniform representation.
>> C makes no such guarantees, and machines _have_ had
>> different types of representations in the past.
>> Some not-so-obsolete 64-bit machines in effect use fat
>> representations for pointers to functions (descriptors),
>> but they usually cheat and use pointers to the descriptors
>> instead. However, a C implementation could legally
>> represent a function pointer as a 128-bit value, while
>> data pointers remain 64 bits.
>
> IIRC for all types foo, sizeof(foo *) <= sizeof(void *), no?
No. There is no implied relation between data pointers and function
pointers. The only requirement is that all _function_ pointers smell
alike, because you can convert any function pointer to any other function
pointer and back without losing information. There is no dedicated
generic function pointer type, any one can function as one.
Andreas.
--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Maxfeldstra?e 5, 90409 N?rnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Mikael Pettersson wrote:
>
> You're assuming pointers have uniform representation.
> C makes no such guarantees, and machines _have_ had
> different types of representations in the past.
>
And are there *any* such architectures for which gcc are targetted. I'm not
advocating advancing this to the C standard, I'm advocating not removing an
extension which has been documented for a decade.
-hpa
Mikael Pettersson wrote:
>
> You're assuming pointers have uniform representation.
> C makes no such guarantees, and machines _have_ had
> different types of representations in the past.
>
By the way, I am not in any shape, way or form making that assumption -
although that's presumably how it would be *implemented* in an architecture
with sane pointers like, to the best of my knowledge ALL gcc targets.
(foo *)bar++;
... should be implemented as ...
({
foo *tmp1 = (foo *)bar;
tmp2 = tmp1 + 1;
bar = __typeof__(bar)tmp2;
tmp1;
})
-hpa
On Tue, 1 Jun 2004, Mikael Pettersson wrote:
>
> You're assuming pointers have uniform representation.
Are we?
I don't see any point where we cast any function pointers to anything
else.
We cast data pointers all over the place, but that is actually guaranteed
to work in C for some "large enough" integer type, and "unsigned long" is
pretty much it.
And even function pointers should be safeish. The fact that some broken
architecture (can you say "ia64"?) has totally idiotic calling conventions
and requires the caller to load the GP value is _their_ problem. The
architecture will either die or hide the fact that it's being silly. For
now it's hiding it.
Repeat after me: practice is more important than theory. A _lot_ more
important.
Linus
On Tue, Jun 01, 2004 at 10:27:16AM -0700, Linus Torvalds wrote:
> And even function pointers should be safeish. The fact that some
> broken architecture (can you say "ia64"?) has totally idiotic
> calling conventions and requires the caller to load the GP value is
> _their_ problem.
ia64 function pointers are actually pointers to the 128-bit
descriptors so all pointers are still 64-bit.
--cw
H. Peter Anvin writes:
> Mikael Pettersson wrote:
> >
> > You're assuming pointers have uniform representation.
> > C makes no such guarantees, and machines _have_ had
> > different types of representations in the past.
> >
>
> By the way, I am not in any shape, way or form making that assumption -
> although that's presumably how it would be *implemented* in an architecture
> with sane pointers like, to the best of my knowledge ALL gcc targets.
>
> (foo *)bar++;
>
> ... should be implemented as ...
>
> ({
> foo *tmp1 = (foo *)bar;
> tmp2 = tmp1 + 1;
> bar = __typeof__(bar)tmp2;
> tmp1;
> })
I did an experiment with updating an unsigned short
via a cast-as-lvalue to unsigned char, and gcc did
in fact implement the temp + copy semantics you
describe above. That is,
unsigned short x = 0xaafe;
((unsigned char)x)++;
resulted in x == 0x00ff.
So cast-as-lvalue is at least reasonably correctly
implemented in gcc.
Whether it's useful and portable is a another matter :-)