[RFA:] sim/common/aclocal.m4: correct duplicate arg test for --enable-sim-hardware=...

[RFA:] sim/common/aclocal.m4: correct duplicate arg test for --enable-sim-hardware=...

[Hans-Peter Nilsson]

The existing test was bogus: there was no variable f, and
no " $f " could match a "x" anyway (missing spaces).  The result
was that you'd always get duplicates and always a build error
for the bogus dv-x.o that was added if you use the
--enable-sim-hardware=... option.  I think the below is what was
intended.  Comma is an appropriate delimeter to use for
combining the "x" exclusion duplication test as it has been
filtered out from $hardware.  For those in doubt, the "or" is
supposedly portable; see other uses in configure, for example the
general --enable-* parse bits at the top.

Tested with patches for cris/configure.ac (adding a dv-rv.c and
"SIM_AC_OPTION_HARDWARE(no,,rv)") and
 --enable-sim-hardware
(enables all base hardware and rv)
 --enable-sim-hardware=rv
(enables rv)
 --enable-sim-hardware=,
(as --enable-sim-hardware)
 --enable-sim-hardware=rv,x,rv,rv,x,x
(as --enable-sim-hardware=rv)

and observing expected behavior on the "Setting hardware to"
line at configure time and that the expected dv-*.o file(s) were
compiled.

Ok to commit?  With obvious rebuild of configure for all users?
sim/common:

	* aclocal.m4 (SIM_AC_OPTION_HARDWARE): Correct duplicate-
	option-contents test.

Index: aclocal.m4
===================================================================
RCS file: /cvs/src/src/sim/common/aclocal.m4,v
retrieving revision 1.11
diff -p -u -r1.11 aclocal.m4
--- aclocal.m4	23 Mar 2005 18:55:14 -0000	1.11
+++ aclocal.m4	25 Mar 2006 15:56:54 -0000
@@ -594,9 +594,8 @@ else
   sim_hw=""
   sim_hw_objs="\$(SIM_COMMON_HW_OBJS)"
   for i in x $hardware ; do
-    case " $f " in
-      x) ;;
-      *" $i "*) ;;
+    case " $sim_hw,$i " in
+      *",x "* | *" $i,"*) ;;
       *) sim_hw="$sim_hw $i" ; sim_hw_objs="$sim_hw_objs dv-$i.o";;
     esac
   done

brgds, H-P

Re: [RFA:] sim/common/aclocal.m4: correct duplicate arg test for --enable-sim-hardware=...

[Daniel Jacobowitz]

On Sat, Mar 25, 2006 at 05:22:58PM +0100, Hans-Peter Nilsson wrote:
> -    case " $f " in
> -      x) ;;
> -      *" $i "*) ;;
> +    case " $sim_hw,$i " in
> +      *",x "* | *" $i,"*) ;;
>        *) sim_hw="$sim_hw $i" ; sim_hw_objs="$sim_hw_objs dv-$i.o";;

I don't think that's right.  Don't you want:

  for i in $hardware ; do
    case " $sim_hw " in
      *" $i "*) ;;
      *) sim_hw="$sim_hw $i" ; sim_hw_objs="$sim_hw_objs dv-$i.o";;
    esac
  done

The "in x $hardware" bit is not necessary (that's only necessary from
Makefiles), and the commas don't work like you'd think, since sim_hw is
space separated.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA:] sim/common/aclocal.m4: correct duplicate arg test for --enable-sim-hardware=...

[Hans-Peter Nilsson]

> Date: Tue, 28 Mar 2006 17:27:21 -0500
> From: Daniel Jacobowitz <drow@false.org>

> On Sat, Mar 25, 2006 at 05:22:58PM +0100, Hans-Peter Nilsson wrote:
> > -    case " $f " in
> > -      x) ;;
> > -      *" $i "*) ;;
> > +    case " $sim_hw,$i " in
> > +      *",x "* | *" $i,"*) ;;
> >        *) sim_hw="$sim_hw $i" ; sim_hw_objs="$sim_hw_objs dv-$i.o";;
> 
> I don't think that's right.  Don't you want:
> 
>   for i in $hardware ; do
>     case " $sim_hw " in
>       *" $i "*) ;;
>       *) sim_hw="$sim_hw $i" ; sim_hw_objs="$sim_hw_objs dv-$i.o";;
>     esac
>   done

Yeah, that would seem the obvious correction if there isn't a
"x" to remove.  Ha!  Consider it done as such! :-)

> The "in x $hardware" bit is not necessary (that's only necessary from
> Makefiles),

Ok then, it's just that the previous author didn't think so, and
so I thought maybe there was a reason for that.

> and the commas don't work like you'd think, since sim_hw is
> space separated.

It's moot now, but that's was the point, as I also mentioned!
In other words, using a comma to join two values for a combined
$i-in-$sim_hw existence and filter-out-x test, with either side
wildcarded out, would be safe, as "," doesn't occur in
$hardware; it's replaced by spaces just a bit higher up; I guess
you saw that.

brgds, H-P

Re: [RFA:] sim/common/aclocal.m4: correct duplicate arg test for --enable-sim-hardware=...

[Daniel Jacobowitz]

On Wed, Mar 29, 2006 at 02:27:29AM +0200, Hans-Peter Nilsson wrote:
> Yeah, that would seem the obvious correction if there isn't a
> "x" to remove.  Ha!  Consider it done as such! :-)

That version's OK.

> Ok then, it's just that the previous author didn't think so, and
> so I thought maybe there was a reason for that.

Lots of people do this out of paranoia, since they get bit by the
Makefile version, I think.

> It's moot now, but that's was the point, as I also mentioned!

Ah, this wasn't explained enough for me, and I misread where the comma
was.  I see now.

-- 
Daniel Jacobowitz
CodeSourcery