2002-09-01 07:10:26

by Nicholas Miell

[permalink] [raw]
Subject: 2.5.33 PNPBIOS does not compile

pnpbios_core.c: In function `call_pnp_bios':
pnpbios_core.c:167: invalid lvalue in unary `&'
pnpbios_core.c:167: invalid lvalue in unary `&'
pnpbios_core.c:169: invalid lvalue in unary `&'
pnpbios_core.c:169: invalid lvalue in unary `&'
pnpbios_core.c: In function `pnpbios_init':
pnpbios_core.c:1276: invalid lvalue in unary `&'
pnpbios_core.c:1276: invalid lvalue in unary `&'
pnpbios_core.c:1277: invalid lvalue in unary `&'
pnpbios_core.c:1277: invalid lvalue in unary `&'
pnpbios_core.c:1278: invalid lvalue in unary `&'
pnpbios_core.c:1278: invalid lvalue in unary `&'
make[2]: *** [pnpbios_core.o] Error 1
make[2]: Target `first_rule' not remade because of errors.
make[1]: *** [pnp] Error 2

... which is the result of the expansion of the Q_SET_SEL and Q2_SET_SEL
macros.

My guess is that changing the macros so that they use set_limit instead
of _set_limit is what broke things, but I don't understand any of this
x86 segmentation voodoo, so I'm not touching it.


2002-09-01 19:14:11

by Luca Barbieri

[permalink] [raw]
Subject: [PATCH] Re: 2.5.33 PNPBIOS does not compile

On Sun, 2002-09-01 at 09:14, Nicholas Miell wrote:
> pnpbios_core.c: In function `call_pnp_bios':
> pnpbios_core.c:167: invalid lvalue in unary `&'
> pnpbios_core.c:167: invalid lvalue in unary `&'
> pnpbios_core.c:169: invalid lvalue in unary `&'
> pnpbios_core.c:169: invalid lvalue in unary `&'
> pnpbios_core.c: In function `pnpbios_init':
> pnpbios_core.c:1276: invalid lvalue in unary `&'
> pnpbios_core.c:1276: invalid lvalue in unary `&'
> pnpbios_core.c:1277: invalid lvalue in unary `&'
> pnpbios_core.c:1277: invalid lvalue in unary `&'
> pnpbios_core.c:1278: invalid lvalue in unary `&'
> pnpbios_core.c:1278: invalid lvalue in unary `&'
> make[2]: *** [pnpbios_core.o] Error 1
> make[2]: Target `first_rule' not remade because of errors.
> make[1]: *** [pnp] Error 2
>
> ... which is the result of the expansion of the Q_SET_SEL and Q2_SET_SEL
> macros.
Yes, this should fix the problem.

diff --exclude-from=/home/ldb/src/linux-exclude -urNdp linux-2.5.32/drivers/pnp/pnpbios_core.c linux-2.5.32_pnpbelow/drivers/pnp/pnpbios_core.c
--- linux-2.5.32/drivers/pnp/pnpbios_core.c 2002-08-27 21:26:32.000000000 +0200
+++ linux-2.5.32_pnpbelow/drivers/pnp/pnpbios_core.c 2002-08-31 18:38:38.000000000 +0200
@@ -127,11 +127,11 @@ __asm__(

#define Q_SET_SEL(cpu, selname, address, size) \
set_base(cpu_gdt_table[cpu][(selname) >> 3], __va((u32)(address))); \
-set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
+set_limit(cpu_gdt_table[cpu][(selname) >> 3], size)

#define Q2_SET_SEL(cpu, selname, address, size) \
set_base(cpu_gdt_table[cpu][(selname) >> 3], (u32)(address)); \
-set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
+set_limit(cpu_gdt_table[cpu][(selname) >> 3], size)

/*
* At some point we want to use this stack frame pointer to unwind


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2002-09-01 20:13:11

by Ray Lee

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.33 PNPBIOS does not compile

Hi there,

> #define Q_SET_SEL(cpu, selname, address, size) \
> set_base(cpu_gdt_table[cpu][(selname) >> 3], __va((u32)(address))); \
> -set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
> +set_limit(cpu_gdt_table[cpu][(selname) >> 3], size)

> #define Q2_SET_SEL(cpu, selname, address, size) \
> set_base(cpu_gdt_table[cpu][(selname) >> 3], (u32)(address)); \
> -set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
> +set_limit(cpu_gdt_table[cpu][(selname) >> 3], size)

These look very wrong. They're not wrapped in the standard do {...}
while(0) protection, and used inside bare if statements below. Can
someone who knows the code verify that these should be wrapped?

(Not, mind you, that I'm complaining about your patch. You didn't
introduce the problem, it just caught my eye.)

Ray

2002-09-01 21:30:56

by Ray Lee

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.33 PNPBIOS does not compile

On Sun, 2002-09-01 at 13:17, Ray Lee wrote:
> These look very wrong. They're not wrapped in the standard do {...}
> while(0) protection, and used inside bare if statements below. Can
> someone who knows the code verify that these should be wrapped?

Back from running errands, and took a longer look at the code. I don't
know if the current form is harmless or not, but it is definitely
incorrect. The patch below corrects the compile failure, as well as the
multi-statement macro defines used in bare if statements; please apply.

Ray

diff -urX ../dontdiff ../linux-2.5.33/drivers/pnp/pnpbios_core.c ./drivers/pnp/pnpbios_core.c
--- ../linux-2.5.33/drivers/pnp/pnpbios_core.c 2002-09-01 09:38:10.000000000 -0700
+++ ./drivers/pnp/pnpbios_core.c 2002-09-01 14:27:55.000000000 -0700
@@ -126,12 +126,16 @@
);

#define Q_SET_SEL(cpu, selname, address, size) \
+do { \
set_base(cpu_gdt_table[cpu][(selname) >> 3], __va((u32)(address))); \
-set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
+set_limit(cpu_gdt_table[cpu][(selname) >> 3], size); \
+} while(0)

#define Q2_SET_SEL(cpu, selname, address, size) \
+do { \
set_base(cpu_gdt_table[cpu][(selname) >> 3], (u32)(address)); \
-set_limit(&cpu_gdt_table[cpu][(selname) >> 3], size)
+set_limit(cpu_gdt_table[cpu][(selname) >> 3], size); \
+} while(0)

/*
* At some point we want to use this stack frame pointer to unwind