Uwe Hermann wrote:
Modified: trunk/LinuxBIOSv2/src/boot/elfboot.c
--- trunk/LinuxBIOSv2/src/boot/elfboot.c 2007-10-23 19:23:52 UTC (rev 2889) +++ trunk/LinuxBIOSv2/src/boot/elfboot.c 2007-10-23 22:17:45 UTC (rev 2890) @@ -144,7 +144,7 @@ { struct verify_callback *cb_chain; unsigned char *note, *end;
- char *program, *version;
- unsigned char *program, *version;
This doesn't look like the correct fix. Strings should always be 'char *' or 'const char *' IMO, so the code where program/version is used should be fixed instead.
Oh, but the code expects an unsigned char, so it should get one. It's not that I cast anything around wildly. This is processing ELF header data, 8bit, so, according to the rule you set up below, this fix should be correct. The fact that in this case the code is parsing a string out of that data is secondary I think. Better fixes or ideas are no doubt highly welcome.
Modified: trunk/LinuxBIOSv2/src/devices/device_util.c
--- trunk/LinuxBIOSv2/src/devices/device_util.c 2007-10-23 19:23:52 UTC (rev 2889) +++ trunk/LinuxBIOSv2/src/devices/device_util.c 2007-10-23 22:17:45 UTC (rev 2890) @@ -454,7 +454,7 @@ void report_resource_stored(device_t dev, struct resource *resource, const char *comment) { if (resource->flags & IORESOURCE_STORED) {
unsigned char buf[10];
char buf[10];If buf contains a string, yes. If it should contain 8bit data, it should be uint8_t or u8.
Ok, then I assume you are saying everything is ok.
#if OPT_HT_LINK == 1 @@ -123,15 +126,17 @@
static int ht_setup_link(struct ht_link *prev, device_t dev, unsigned pos) { +#if OPT_HT_LINK == 1 static const uint8_t link_width_to_pow2[]= { 3, 4, 0, 5, 1, 2, 0, 0 }; static const uint8_t pow2_to_link_width[] = { 0x7, 4, 5, 0, 1, 3 };
- struct ht_link cur[1]; unsigned present_width_cap, upstream_width_cap; unsigned present_freq_cap, upstream_freq_cap; unsigned ln_present_width_in, ln_upstream_width_in; unsigned ln_present_width_out, ln_upstream_width_out; unsigned freq, old_freq; unsigned present_width, upstream_width, old_width;
+#endif
- struct ht_link cur[1];
This change is not trivial, as it might very well have effects on the run-time behaviour of LinuxBIOS / the hardware.
Can we say for sure that there's no reason to (re-)initialize here? Is this tested on hardware?
Oh yes, this is highly trivial. Look at the code, all these are unused variables. The produced code does not change. The OPT_HT_LINK define is tested already below, where the real magic happens. But when this was introduced, all the variables were left unused.
Stefan