1. gcc doesn't like a character array being cast to a struct.
-extern unsigned char AmlCode[]; +extern void* AmlCode;
2. Include <console/console.h> if you're using PRINTK_IN_CAR (not ROMCC) 3. Add a few more "nuisance" prototypes.
Signed-off-by: Myles Watson mylesgw@gmail.com
I think it would be nice to add the number of warnings for each board on one of the build pages. It looks like there are many that could compile without any warnings, and some that look like they have a few too many. Here's the list after this patch:
coreboot-builds/a-trend_atc-6220/make.log:2 coreboot-builds/a-trend_atc-6240/make.log:2 coreboot-builds/abit_be6-ii_v2_0/make.log:2 coreboot-builds/advantech_pcm-5820/make.log:76 coreboot-builds/amd_db800/make.log:111 coreboot-builds/amd_dbm690t/make.log:132 coreboot-builds/amd_mahogany/make.log:182 coreboot-builds/amd_mahogany_fam10/make.log:378 coreboot-builds/amd_norwich/make.log:110 coreboot-builds/amd_pistachio/make.log:127 coreboot-builds/amd_rumba/make.log:65 coreboot-builds/amd_serengeti_cheetah/make.log:74 coreboot-builds/amd_serengeti_cheetah_fam10/make.log:270 coreboot-builds/arima_hdama/make.log:49 coreboot-builds/artecgroup_dbe61/make.log:112 coreboot-builds/asi_mb_5blgp/make.log:2 coreboot-builds/asi_mb_5blmp/make.log:2 coreboot-builds/asus_a8n_e/make.log:54 coreboot-builds/asus_a8v-e_se/make.log:74 coreboot-builds/asus_m2v-mx_se/make.log:103 coreboot-builds/asus_mew-am/make.log:9 coreboot-builds/asus_mew-vm/make.log:10 coreboot-builds/asus_p2b-d/make.log:3 coreboot-builds/asus_p2b-ds/make.log:3 coreboot-builds/asus_p2b-f/make.log:2 coreboot-builds/asus_p2b-ls/make.log:2 coreboot-builds/asus_p2b/make.log:2 coreboot-builds/asus_p3b-f/make.log:2 coreboot-builds/axus_tc320/make.log:2 coreboot-builds/azza_pt-6ibd/make.log:2 coreboot-builds/bcom_winnet100/make.log:2 coreboot-builds/bcom_winnetp680/make.log:100 coreboot-builds/biostar_m6tba/make.log:2 coreboot-builds/broadcom_blast/make.log:78 coreboot-builds/compaq_deskpro_en_sff_p600/make.log:2 coreboot-builds/dell_s1850/make.log:5 coreboot-builds/digitallogic_adl855pc/make.log:10 coreboot-builds/digitallogic_msm586seg/make.log:12 coreboot-builds/digitallogic_msm800sev/make.log:111 coreboot-builds/eaglelion_5bcm/make.log:2 coreboot-builds/emulation_qemu-x86/make.log:0 coreboot-builds/gigabyte_ga-6bxc/make.log:2 coreboot-builds/gigabyte_ga_2761gxdk/make.log:128 coreboot-builds/gigabyte_m57sli/make.log:133 coreboot-builds/hp_dl145_g3/make.log:110 coreboot-builds/hp_e_vectra_p2706t/make.log:9 coreboot-builds/ibm_e325/make.log:48 coreboot-builds/ibm_e326/make.log:50 coreboot-builds/iei_juki-511p/make.log:2 coreboot-builds/iei_nova4899r/make.log:2 coreboot-builds/iei_pcisa-lx-800-r10/make.log:111 coreboot-builds/intel_d945gclf/make.log:24 coreboot-builds/intel_eagleheights/make.log:49 coreboot-builds/intel_jarrell/make.log:13 coreboot-builds/intel_mtarvon/make.log:6 coreboot-builds/intel_truxton/make.log:6 coreboot-builds/intel_xe7501devkit/make.log:15 coreboot-builds/iwill_dk8_htx/make.log:51 coreboot-builds/iwill_dk8s2/make.log:58 coreboot-builds/iwill_dk8x/make.log:53 coreboot-builds/jetway_j7f24/make.log:99 coreboot-builds/kontron_986lcd-m/make.log:15 coreboot-builds/kontron_kt690/make.log:131 coreboot-builds/lippert_frontrunner/make.log:42 coreboot-builds/lippert_roadrunner-lx/make.log:113 coreboot-builds/lippert_spacerunner-lx/make.log:114 coreboot-builds/mitac_6513wu/make.log:9 coreboot-builds/msi_ms6119/make.log:2 coreboot-builds/msi_ms6147/make.log:2 coreboot-builds/msi_ms6156/make.log:2 coreboot-builds/msi_ms6178/make.log:9 coreboot-builds/msi_ms7135/make.log:58 coreboot-builds/msi_ms7260/make.log:131 coreboot-builds/msi_ms9185/make.log:108 coreboot-builds/msi_ms9282/make.log:132 coreboot-builds/msi_ms9652_fam10/make.log:298 coreboot-builds/nec_powermate2000/make.log:9 coreboot-builds/newisys_khepri/make.log:48 coreboot-builds/nvidia_l1_2pvv/make.log:135 coreboot-builds/olpc_btest/make.log:72 coreboot-builds/olpc_rev_a/make.log:71 coreboot-builds/pcengines_alix1c/make.log:112 coreboot-builds/rca_rm4100/make.log:1 coreboot-builds/roda_rk886ex/make.log:25 coreboot-builds/soyo_sy-6ba-plus-iii/make.log:2 coreboot-builds/sunw_ultra40/make.log:59 coreboot-builds/supermicro_h8dme/make.log:126 coreboot-builds/supermicro_h8dmr/make.log:124 coreboot-builds/supermicro_h8dmr_fam10/make.log:293 coreboot-builds/supermicro_h8qme_fam10/make.log:293 coreboot-builds/supermicro_x6dai_g/make.log:13 coreboot-builds/supermicro_x6dhe_g/make.log:17 coreboot-builds/supermicro_x6dhe_g2/make.log:9 coreboot-builds/supermicro_x6dhr_ig/make.log:6 coreboot-builds/supermicro_x6dhr_ig2/make.log:6 coreboot-builds/technexion_tim5690/make.log:130 coreboot-builds/technexion_tim8690/make.log:130 coreboot-builds/technologic_ts5300/make.log:10 coreboot-builds/televideo_tc7020/make.log:2 coreboot-builds/thomson_ip1000/make.log:0 coreboot-builds/tyan_s1846/make.log:2 coreboot-builds/tyan_s2735/make.log:78 coreboot-builds/tyan_s2850/make.log:52 coreboot-builds/tyan_s2875/make.log:50 coreboot-builds/tyan_s2880/make.log:54 coreboot-builds/tyan_s2881/make.log:53 coreboot-builds/tyan_s2882/make.log:51 coreboot-builds/tyan_s2885/make.log:49 coreboot-builds/tyan_s2891/make.log:58 coreboot-builds/tyan_s2892/make.log:4 coreboot-builds/tyan_s2895/make.log:49 coreboot-builds/tyan_s2912/make.log:136 coreboot-builds/tyan_s2912_fam10/make.log:298 coreboot-builds/tyan_s4880/make.log:53 coreboot-builds/tyan_s4882/make.log:44 coreboot-builds/via_epia-cn/make.log:48 coreboot-builds/via_epia-m/make.log:135 coreboot-builds/via_epia-m700/make.log:204 coreboot-builds/via_epia-n/make.log:132 coreboot-builds/via_epia/make.log:62 coreboot-builds/via_pc2500e/make.log:45 coreboot-builds/via_vt8454c/make.log:60 coreboot-builds/winent_pl6064/make.log:110
Thanks, Myles
On 3/17/10 10:33 PM, Myles Watson wrote:
+#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif
I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__ did..
Acked-by: Stefan Reinauer stepan@coresystems.de
On 3/17/10 11:11 PM, Stefan Reinauer wrote:
On 3/17/10 10:33 PM, Myles Watson wrote:
+#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif
I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__ did..
Acked-by: Stefan Reinauer stepan@coresystems.de
Sorry, got to N-Ack again....
unsigned char AmlCode[] = { 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 "DSDT.'.." */ 0x02,0x7C,0x41,0x4D,0x44,0x20,0x20,0x20, /* 00000008 ".|AMD " */ 0x4D,0x41,0x48,0x4F,0x47,0x41,0x4E,0x59, /* 00000010 "MAHOGANY" */ .... }
and in another file it looks like this:
-extern unsigned char AmlCode[]; +extern void* AmlCode;
So AmlCode is used as a void * pointing to the address 0x54445344
which is definitely not what you want...
Even though it might appear that char *foo and char foo[] are the same in C, they're in fact not.
Stefan
On Wed, Mar 17, 2010 at 4:16 PM, Stefan Reinauer stepan@coresystems.dewrote:
On 3/17/10 11:11 PM, Stefan Reinauer wrote:
On 3/17/10 10:33 PM, Myles Watson wrote:
+#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif
I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__
did..
Acked-by: Stefan Reinauer stepan@coresystems.de
Sorry, got to N-Ack again....
unsigned char AmlCode[] = { 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 "DSDT.'.." */ 0x02,0x7C,0x41,0x4D,0x44,0x20,0x20,0x20, /* 00000008 ".|AMD " */ 0x4D,0x41,0x48,0x4F,0x47,0x41,0x4E,0x59, /* 00000010 "MAHOGANY" */ .... }
and in another file it looks like this:
-extern unsigned char AmlCode[]; +extern void* AmlCode;
So AmlCode is used as a void * pointing to the address 0x54445344
which is definitely not what you want...
You're right. And that's definitely not what I expected to happen. It seems like there ought to be a correct way to do this. I'll keep looking.
Thanks, Myles
Even though it might appear that char *foo and char foo[] are the same in C, they're in fact not.
Stefan
-- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: info@coresystems.de • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On 3/18/10 12:05 AM, Myles Watson wrote:
unsigned char AmlCode[] = { 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 "DSDT.'.." */
and in another file it looks like this: -extern unsigned char AmlCode[]; +extern void* AmlCode; So AmlCode is used as a void * pointing to the address 0x54445344 which is definitely not what you want...You're right. And that's definitely not what I expected to happen. It seems like there ought to be a correct way to do this. I'll keep looking.
What is it, you want to do?
-extern unsigned char AmlCode[]; +extern void* AmlCode;
So AmlCode is used as a void * pointing to the address 0x54445344
which is definitely not what you want...
You're right. And that's definitely not what I expected to happen. It seems like there ought to be a correct way to do this. I'll keep looking.
What is it, you want to do?
Get rid of the type punned warning that gcc gives for that code where AmlCode gets cast to acpi_header_t.
Thanks, Myles
Hi Segher,
you're probably able to solve this one easily.
On 18.03.2010 00:45, Myles Watson wrote:
-extern unsigned char AmlCode[]; +extern void* AmlCode;
So AmlCode is used as a void * pointing to the address 0x54445344
which is definitely not what you want...
You're right. And that's definitely not what I expected to happen. It seems like there ought to be a correct way to do this. I'll keep looking.
What is it, you want to do?
Get rid of the type punned warning that gcc gives for that code where AmlCode gets cast to acpi_header_t.
Could __attribute__((may_alias)) help?
Then again, a Google search turns up a few miscompiles with that attribute, so another option may be to run memcpy(AmlCode,AmlCode, ...).
Regards, Carl-Daniel
On Wed, Mar 17, 2010 at 6:17 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
Hi Segher,
you're probably able to solve this one easily.
On 18.03.2010 00:45, Myles Watson wrote:
-extern unsigned char AmlCode[]; +extern void* AmlCode;
So AmlCode is used as a void * pointing to the address 0x54445344
What is it, you want to do?
Get rid of the type punned warning that gcc gives for that code where AmlCode gets cast to acpi_header_t.
Could __attribute__((may_alias)) help?
I'd rather not use an attribute if we can just use a cast.
It seems to work to have an intermediate void*:
void_ptr = &AmlCode_ssdt; current += ((acpi_header_t *)void_ptr)->length;
It compiles without warning and is functionally correct, but it may be too ugly. I'm still surprised that it needs to be &AmlCode_ssdt. I really expected Amlcode_ssdt to be a cast-able pointer.
Thanks, Myles
Myles Watson wrote:
On Wed, Mar 17, 2010 at 6:17 PM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net mailto:c-d.hailfinger.devel.2006@gmx.net> wrote:
Hi Segher, you're probably able to solve this one easily. On 18.03.2010 00:45, Myles Watson wrote: >>> -extern unsigned char AmlCode[]; >>> +extern void* AmlCode; >>> >>> >>> So AmlCode is used as a void * pointing to the address 0x54445344 >> What is it, you want to do? >> >> > Get rid of the type punned warning that gcc gives for that code where > AmlCode gets cast to acpi_header_t. > Could __attribute__((may_alias)) help?I'd rather not use an attribute if we can just use a cast.
It seems to work to have an intermediate void*:
void_ptr = &AmlCode_ssdt; current += ((acpi_header_t *)void_ptr)->length;It compiles without warning and is functionally correct, but it may be too ugly. I'm still surprised that it needs to be &AmlCode_ssdt. I really expected Amlcode_ssdt to be a cast-able pointer.
If you have this in one C file:
static const char AmlCode[] = {0,1,2,3,4,5,6,7};
You can then have this in another C file, which is what you want:
struct AmlCode_s { /* stuff ? */ int a, b; };
extern const struct AmlCode_s AmlCode;
This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that.
(Image what would happen if you were to replace the first file with assembler.)
I'm not sure I like this solution.
MM
Mark Marshall wrote:
If you have this in one C file:
static const char AmlCode[] = {0,1,2,3,4,5,6,7};
const char AmlCode[] = {0,1,2,3,4,5,6,7};
Doh!
On 3/18/10 11:35 AM, Mark Marshall wrote:
This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that.
Since the type punning issue is about wrong alignment, I think the C compiler does not do the right thing in that case, we just trick it around the warning.
Stefan
This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that.
Since the type punning issue is about wrong alignment, I think the C compiler does not do the right thing in that case, we just trick it around the warning.
So it seems like we should make sure the alignment is correct and then trick it around the warning, right?
What's the best way to make sure that a u8 array is aligned on a four-byte boundary?
Thanks, Myles
On 18.03.2010 15:05, Stefan Reinauer wrote:
On 3/18/10 11:35 AM, Mark Marshall wrote:
This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that.
Since the type punning issue is about wrong alignment, I think the C compiler does not do the right thing in that case, we just trick it around the warning.
AFAIK (and I could be wrong) type punning is not only about alignment, but the C compiler is free to optimize the generated code in a way that may break your expectations if you're violating strict aliasing rules.
Regards, Carl-Daniel
I split the patch into two. One has the acpi changes.
Abuild-tested and boot tested on serengeti_cheetah_fam10.
If we get rid of the SSE2 warnings, we could have about 30 boards compiling without any warnings!
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On Thu, Mar 18, 2010 at 11:28 AM, Myles Watson mylesgw@gmail.com wrote:
I split the patch into two. One has the acpi changes.
For the ACPI struct, the safest way would probably be to copy sizeof(acpi_header_t) bytes into a temp header, then copy the full temp->size. I think we can probably trust the compiler to give us 4-byte alignment, but maybe we should add an alignment attribute when we change the name.
Thanks, Myles
Abuild-tested and boot tested on serengeti_cheetah_fam10.
If we get rid of the SSE2 warnings, we could have about 30 boards compiling without any warnings!
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
Am 18.03.2010 18:28, schrieb Myles Watson:
I split the patch into two. One has the acpi changes.
Abuild-tested and boot tested on serengeti_cheetah_fam10.
If we get rid of the SSE2 warnings, we could have about 30 boards compiling without any warnings!
Signed-off-by: Myles Watson <mylesgw@gmail.com mailto:mylesgw@gmail.com>
The non-ACPI warning stuff is Acked-by: Patrick Georgi patrick.georgi@coresystems.de
I didn't look into the ACPI stuff yet.
As for SSE2 warnings, just add a "default n" to its global definition in src/cpu/Kconfig? (and to MMX and SSE, too)
That's also Acked-by: Patrick Georgi patrick.georgi@coresystems.de
Regards, Patrick
Signed-off-by: Myles Watson <mylesgw@gmail.com
mailto:mylesgw@gmail.com> The non-ACPI warning stuff is Acked-by: Patrick Georgi patrick.georgi@coresystems.de
I didn't look into the ACPI stuff yet.
As for SSE2 warnings, just add a "default n" to its global definition in src/cpu/Kconfig? (and to MMX and SSE, too)
I just did SSE2, since it is only for performance of ramtest. I haven't looked much at MMX and SSE to see where they're used.
That's also Acked-by: Patrick Georgi patrick.georgi@coresystems.de
Rev 5260.
Thanks, Myles
Ping.
I've inlined a representative snippet. Not only does it get rid of the warnings, it gets rid of a lot of casting.
Thanks, Myles
Index: svn/src/mainboard/tyan/s2895/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/tyan/s2895/acpi_tables.c +++ svn/src/mainboard/tyan/s2895/acpi_tables.c @@ -20,7 +20,7 @@ #include <../../../northbridge/amd/amdk8/amdk8_acpi.h> #include <cpu/amd/model_fxx_powernow.h>
-extern unsigned char AmlCode[]; +extern acpi_header_t AmlCode;
unsigned long acpi_fill_mcfg(unsigned long current) { @@ -164,8 +164,8 @@ unsigned long write_acpi_tables(unsigned current = ALIGN(current, 16); dsdt = (acpi_header_t *) current; printk_debug("ACPI: * DSDT %p\n", dsdt); - current += ((acpi_header_t *) AmlCode)->length; - memcpy((void*) dsdt, (void*)AmlCode, ((acpi_header_t*)AmlCode)->length); + current += AmlCode.length; + memcpy((void*) dsdt, &AmlCode, AmlCode.length); printk_debug("ACPI: * DSDT @ %p Length %x\n",dsdt,dsdt->length);
current = ALIGN(current, 16);
Here's an updated patch which takes care of printk_... -> printk(BIOS_... conversions.
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On 3/24/10 9:50 PM, Myles Watson wrote:
Here's an updated patch which takes care of printk_... -> printk(BIOS_... conversions.
Signed-off-by: Myles Watson <mylesgw@gmail.com mailto:mylesgw@gmail.com>
Thanks, Myles
Defining AmlCode differently in different source files is a bit ugly... My variant of creating an additional void * to do the casting is not exactly beautiful either...
However, Acked-by: Stefan Reinauer stepan@coresystems.de
Defining AmlCode differently in different source files is a bit ugly... My variant of creating an additional void * to do the casting is not exactly beautiful either...
Before I commit, should we make them const too? Maybe that would help people not misuse them?
However, Acked-by: Stefan Reinauer stepan@coresystems.destepan@coresystems.de
Thanks, Myles
On 3/24/10 10:17 PM, Myles Watson wrote:
Defining AmlCode differently in different source files is a bit ugly... My variant of creating an additional void * to do the casting is not exactly beautiful either...Before I commit, should we make them const too? Maybe that would help people not misuse them?
If they're not runtime patched that'd be a good idea... I think on i945 we still patch them up, so const wont work there...
Stefan
Am 24.03.2010 22:20, schrieb Stefan Reinauer:
If they're not runtime patched that'd be a good idea... I think on i945 we still patch them up, so const wont work there...
We patch up the resulting (ie. copied) ACPI data, or not? So the original AmlCode can be const
Patrick
On Wed, Mar 24, 2010 at 3:22 PM, Patrick Georgi patrick@georgi-clan.dewrote:
Am 24.03.2010 22:20, schrieb Stefan Reinauer:
If they're not runtime patched that'd be a good idea... I think on i945 we
still patch them up, so const wont work there...
We patch up the resulting (ie. copied) ACPI data, or not? So the original AmlCode can be const
Yep. Abuild tested, so I committed it. I just had to add const to the pointer used for copying multiple SSDTs.
Rev 5286.
Thanks, Myles
On 3/18/10 3:47 PM, Carl-Daniel Hailfinger wrote:
On 18.03.2010 15:05, Stefan Reinauer wrote:
On 3/18/10 11:35 AM, Mark Marshall wrote:
This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that.
Since the type punning issue is about wrong alignment, I think the C compiler does not do the right thing in that case, we just trick it around the warning.
AFAIK (and I could be wrong) type punning is not only about alignment, but the C compiler is free to optimize the generated code in a way that may break your expectations if you're violating strict aliasing rules.
That's what I read over and over, too but I can't make much sense of it... what does it mean? What are we not supposed to expect? What is it about, more than alignment?
On Sun, Mar 21, 2010 at 01:04:44AM +0100, Stefan Reinauer wrote:
On 3/18/10 3:47 PM, Carl-Daniel Hailfinger wrote:
AFAIK (and I could be wrong) type punning is not only about alignment, but the C compiler is free to optimize the generated code in a way that may break your expectations if you're violating strict aliasing rules.
That's what I read over and over, too but I can't make much sense of it... what does it mean? What are we not supposed to expect? What is it about, more than alignment?
It's about pointer aliases. Let's say one had code like:
void myfunc(u16 *s, u32 *l) { printf("%d", *l); *s += 1; *l += 1; printf("%d", *l); }
Then gcc is free to assume that *s and *l don't point to the same memory location. Thus, myfunc((u16*)&myint, &myint) could give unexpected results.
The gcc warnings point out risky looking casts, but it is not exhaustive. Fixing all the warnings will not prevent all aliasing issues.
Also, I didn't think alignment made any real impact on embedded x86 - as I didn't think gcc would emit any code that required alignment.
-Kevin
On 3/21/10 4:57 PM, Kevin O'Connor wrote:
It's about pointer aliases. Let's say one had code like:
void myfunc(u16 *s, u32 *l) { printf("%d", *l); *s += 1; *l += 1; printf("%d", *l); }
Then gcc is free to assume that *s and *l don't point to the same memory location.
Very obviously they don't. But why would that prevent us from casting a char [] to struct acpi_hdr?
Stefan
On Sun, Mar 21, 2010 at 05:27:31PM +0100, Stefan Reinauer wrote:
On 3/21/10 4:57 PM, Kevin O'Connor wrote:
It's about pointer aliases. Let's say one had code like:
void myfunc(u16 *s, u32 *l) { printf("%d", *l); *s += 1; *l += 1; printf("%d", *l); }
Then gcc is free to assume that *s and *l don't point to the same memory location.
Very obviously they don't. But why would that prevent us from casting a char [] to struct acpi_hdr?
The cast was from an "unsigned char []" - which is actually important as "char *" is treated specially while "unsigned char *" is not.
Otherwise, the compiler didn't stop the cast - it's just generating a warning that it's dangerous. (That is, it's letting the user know that any memory accesses via the "unsigned char *" reference wont necessarily be seen from the "struct acpi_hdr *" reference and vice-versa.)
Did I miss something?
-Kevin
On 3/21/10 5:38 PM, Kevin O'Connor wrote:
Otherwise, the compiler didn't stop the cast - it's just generating a warning that it's dangerous. (That is, it's letting the user know that any memory accesses via the "unsigned char *" reference wont necessarily be seen from the "struct acpi_hdr *" reference and vice-versa.)
So making both (or even one of them) const should help?
On Sun, Mar 21, 2010 at 06:23:43PM +0100, Stefan Reinauer wrote:
On 3/21/10 5:38 PM, Kevin O'Connor wrote:
Otherwise, the compiler didn't stop the cast - it's just generating a warning that it's dangerous. (That is, it's letting the user know that any memory accesses via the "unsigned char *" reference wont necessarily be seen from the "struct acpi_hdr *" reference and vice-versa.)
So making both (or even one of them) const should help?
I don't know. It's probably worth testing. A const pointer doesn't mean the memory can't change - it just means the memory can't be changed via that pointer - so, gcc might still warn.
To be honest, I just turned off the warning in SeaBIOS (-Wno-strict-aliasing). Last time I looked, different versions of the compiler had different heuristics for warning, and I couldn't see anything that I was doing that was wrong. So, I'm probably not the best person to ask on how to prevent the warnings. :-)
-Kevin
On 3/17/10 10:33 PM, Myles Watson wrote:
-extern unsigned char AmlCode[]; +extern void* AmlCode;
Is this really a void *?
Did you test that this works?
-
Carl-Daniel Hailfinger -
Kevin O'Connor -
Mark Marshall -
Myles Watson -
Patrick Georgi -
Patrick Georgi -
Stefan Reinauer