eax is used for offset calculation but is also a local
variable used to store register value.
Use val for value and defval for default value to remove
this ambiguity.
Thanks to Julia Lawall for Coccinelle scripting support.
Signed-off-by: Fabian Frederick <[email protected]>
---
tools/lguest/lguest.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index e440524..d466c79 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -1611,12 +1611,12 @@ static void emulate_insn(const u8 insn[])
{
unsigned long args[] = { LHREQ_TRAP, 13 };
unsigned int insnlen = 0, in = 0, small_operand = 0, byte_access;
- unsigned int eax, port, mask;
+ unsigned int val, port, mask;
/*
* Default is to return all-ones on IO port reads, which traditionally
* means "there's nothing there".
*/
- u32 val = 0xFFFFFFFF;
+ u32 defval = 0xFFFFFFFF;
/*
* This must be the Guest kernel trying to do something, not userspace!
@@ -1692,29 +1692,29 @@ static void emulate_insn(const u8 insn[])
* If it was an "IN" instruction, they expect the result to be read
* into %eax, so we change %eax.
*/
- eax = getreg(eax);
+ val = getreg(eax);
if (in) {
/* This is the PS/2 keyboard status; 1 means ready for output */
if (port == 0x64)
- val = 1;
+ defval = 1;
else if (is_pci_addr_port(port))
- pci_addr_ioread(port, mask, &val);
+ pci_addr_ioread(port, mask, &defval);
else if (is_pci_data_port(port))
- pci_data_ioread(port, mask, &val);
+ pci_data_ioread(port, mask, &defval);
/* Clear the bits we're about to read */
- eax &= ~mask;
- /* Copy bits in from val. */
- eax |= val & mask;
+ val &= ~mask;
+ /* Copy bits in from defval. */
+ val |= defval & mask;
/* Now update the register. */
- setreg(eax, eax);
+ setreg(eax, val);
} else {
if (is_pci_addr_port(port)) {
- if (!pci_addr_iowrite(port, mask, eax))
+ if (!pci_addr_iowrite(port, mask, val))
goto bad_io;
} else if (is_pci_data_port(port)) {
- if (!pci_data_iowrite(port, mask, eax))
+ if (!pci_data_iowrite(port, mask, val))
goto bad_io;
}
/* There are many other ports, eg. CMOS clock, serial
@@ -1722,7 +1722,7 @@ static void emulate_insn(const u8 insn[])
}
verbose("IO %s of %x to %u: %#08x\n",
- in ? "IN" : "OUT", mask, port, eax);
+ in ? "IN" : "OUT", mask, port, val);
skip_insn:
/* Finally, we've "done" the instruction, so move past it. */
setreg(eip, getreg(eip) + insnlen);
--
2.4.2
Fabian Frederick <[email protected]> writes:
> eax is used for offset calculation but is also a local
> variable used to store register value.
Well, it's the value we're going to put back in eax. And it came from
eax.
Renaming val to def_val is weird, since it's not the default. You can
tell it's set in various places.
I don't think this patch adds clarity. What are you trying to do?
Thanks,
Rusty.
> tools/lguest/lguest.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
> index e440524..d466c79 100644
> --- a/tools/lguest/lguest.c
> +++ b/tools/lguest/lguest.c
> @@ -1611,12 +1611,12 @@ static void emulate_insn(const u8 insn[])
> {
> unsigned long args[] = { LHREQ_TRAP, 13 };
> unsigned int insnlen = 0, in = 0, small_operand = 0, byte_access;
> - unsigned int eax, port, mask;
> + unsigned int val, port, mask;
> /*
> * Default is to return all-ones on IO port reads, which traditionally
> * means "there's nothing there".
> */
> - u32 val = 0xFFFFFFFF;
> + u32 defval = 0xFFFFFFFF;
>
> /*
> * This must be the Guest kernel trying to do something, not userspace!
> @@ -1692,29 +1692,29 @@ static void emulate_insn(const u8 insn[])
> * If it was an "IN" instruction, they expect the result to be read
> * into %eax, so we change %eax.
> */
> - eax = getreg(eax);
> + val = getreg(eax);
>
> if (in) {
> /* This is the PS/2 keyboard status; 1 means ready for output */
> if (port == 0x64)
> - val = 1;
> + defval = 1;
> else if (is_pci_addr_port(port))
> - pci_addr_ioread(port, mask, &val);
> + pci_addr_ioread(port, mask, &defval);
> else if (is_pci_data_port(port))
> - pci_data_ioread(port, mask, &val);
> + pci_data_ioread(port, mask, &defval);
>
> /* Clear the bits we're about to read */
> - eax &= ~mask;
> - /* Copy bits in from val. */
> - eax |= val & mask;
> + val &= ~mask;
> + /* Copy bits in from defval. */
> + val |= defval & mask;
> /* Now update the register. */
> - setreg(eax, eax);
> + setreg(eax, val);
> } else {
> if (is_pci_addr_port(port)) {
> - if (!pci_addr_iowrite(port, mask, eax))
> + if (!pci_addr_iowrite(port, mask, val))
> goto bad_io;
> } else if (is_pci_data_port(port)) {
> - if (!pci_data_iowrite(port, mask, eax))
> + if (!pci_data_iowrite(port, mask, val))
> goto bad_io;
> }
> /* There are many other ports, eg. CMOS clock, serial
> @@ -1722,7 +1722,7 @@ static void emulate_insn(const u8 insn[])
> }
>
> verbose("IO %s of %x to %u: %#08x\n",
> - in ? "IN" : "OUT", mask, port, eax);
> + in ? "IN" : "OUT", mask, port, val);
> skip_insn:
> /* Finally, we've "done" the instruction, so move past it. */
> setreg(eip, getreg(eip) + insnlen);
> --
> 2.4.2