Re: Simulator patches for dv-sockser.o

[Mike Frysinger <vapier at gentoo dot org>]

On Thursday 21 March 2013 11:53:13 Joel Sherrill wrote:
> Per conversations with Mike Frysinger, I have made other modifications.
> These basically address:
> 
> + simulators which can't work without dv-sockser.o fail at configuration
> + simulators which fail with --disable-sim-hardware fail at configuration
> + added conditionals and FIXME for dv-sockser.o references in source
> 
> Attached is a tarball from git which has a patch per simulator directory
> touched. The last patch is the regeneration of configure's.
> 
> I have built all of the impacted CPUs with --enable-sim-hardware
> and --disable-sim-hardware.

the main idea looks good to me.  just style nits below.

> 0001-sim-common-acinclude.m4-Address-always-required-hard.patch
>
> +  if test "[$1]" = "always"; then
> +    AC_MSG_ERROR([Sorry, but this simulator requires that hardware support 
be enabled. Please configure without --disable-hw-support.])
> +  fi

that line is too long.  just put a new line in the middle and that should be 
fine (since the output will still look fine).
  AC_MSG_ERROR([...................
............])

> 0002-bfin-configure.ac-Address-use-of-dv-sockser.o.patch
>
> +2013-03-20  Joel Sherrill <joel.sherrill@oarcorp.com>

should be two spaces between your name & e-mail.  you should check all the 
ChangeLog entries accordingly.

> +	* configure.ac: Use $SIM_DV_SOCKSER_O

should have a period at the end.  you should check all the ChangeLog entries 
accordingly.

> 0003-sim-mips-Address-use-of-dv-sockser.o.patch
>
> +		AC_MSG_ERROR([Sorry, but tx3904sio hardware support is unavailable 
for your target.  Please use --disable-sim-hardware, or pass a list of devices 
to enable that does not include that.])

same comment re-too long of a line.

also, i think this indents with a tab instead of two spaces ?  a few of the 
patches have that, so you should grep for tabs to make sure none are left in.

> 0005-frv-configure.ac-Address-use-of-dv-sockser.o.patch
>
> +	AC_MSG_ERROR([Sorry, but hardware support in this simulator 
unconditionally relies on dv-sockser.o, it is unavailable for your host. 
Please fix this simulator.])

same comment about too long.  also, change "..., it is" to "... which is".

> 0006-iq2000-configure.ac-Address-use-of-dv-sockser.o.patch
> 0007-m32r-configure.ac-Address-use-of-dv-sockser.o.patch
> 0008-mn10300-configure.ac-Address-use-of-dv-sockser.o.patch
> 0009-sh64-configure.ac-Address-use-of-dv-sockser.o.patch

same feedback as 0005-frv patch above
-mike

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