Re: [RFC] TI MSP430 simulator

[Mike Frysinger <vapier at gentoo dot org>]

On Wednesday 22 May 2013 08:17:13 Nick Clifton wrote:
>   [This is another piece of code that was written by DJ, but which is
>   being contributed whilst he is on vacation].
> 
>   I have attached a patch which I hope addresses all of your concerns.
>   Actually make that "most of your concerns".  The issue of code
>   duplication in files like trace.c has not been addressed.  I agree
>   that this is a sad state of affairs.  It looks to me like the
>   simulator sources as a whole could use a good spring clean.

just style nits now.  no need to round trip if you just plan on fixing them and 
committing.

> # List of object files, less common parts.
> SIM_OBJS = \
>         $(SIM_NEW_COMMON_OBJS) \
>         msp430-sim.o \
> 	trace.o \
>         sim-cpu.o \

indentation is inconsistent with trace.o

> static int
> msp430_reg_fetch (SIM_CPU *cpu, int regno, unsigned char *buf, int len)
> {
>   if (0 <= regno && regno < 16)
>     {
>       if (len == 2)
> 	{
> 	  int val = cpu->state.regs[regno];
> 	  buf[0] = val & 0xff;
> 	  buf[1] = (val >> 8) & 0xff;
> 	  return 0;
> 	}
>       else if (len == 4)
>         {
> 	  int val = cpu->state.regs[regno];

you've got a few places where you're still using 8 or more spaces in a row 
rather than converting to a tab.  might want to just grep for 8 consecutive 
spaces and replace what you find.

> static int
> bcd_to_binary (int v)
> {
>   int r = (  ((v >>  0) & 0xf) * 1
> 	  + ((v >>  4) & 0xf) * 10

i think you meant to have the first line match the latter ones, but you added 
one too many spaces

> static int
> binary_to_bcd (int v)
> {
>   int r = ( ((v /    1) % 10) <<  0
> 	 | ((v /   10) % 10) <<  4

same here
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.