[PATCH] gdb/testsuite: do not hard-code location indices in condbreak-multi-context.exp

[Simon Marchi]

On 2020-10-30 11:06 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> +proc find_location_contexts {} {
> +    global loc_name loc_index bpnum1 fill
> +    global decimal hex gdb_prompt
> +
> +    gdb_test_multiple "info breakpoint $bpnum1" "find_location_indices" {
> +	-re "stop only if ${fill}\r\n" {
> +	    exp_continue
> +	}
> +	-re "^${bpnum1}\.($decimal) ${fill} ${hex} in (A|Base|C)::func${fill}\r\n" {
> +	    set index $expect_out(1,string)
> +	    set name $expect_out(2,string)
> +	    set loc_name($index) $name
> +	    set loc_index($name) $index
> +	    exp_continue
> +	}
> +	-re "$gdb_prompt $" {
> +	    verbose -log "Loc names: $loc_name(1), $loc_name(2), $loc_name(3)"
> +	    if {($loc_name(1) != $loc_name(2))
> +		&& ($loc_name(1) != $loc_name(3))

Is the goal to check that all elements are unique?  If so, should you
also check that $loc_name(2) != $loc_name(3)?  If/when we ever have more
than three elements, it will become interesting to make two for loops
that compare all pairs of elements.

> +		&& ($loc_name(1) != "")
> +		&& ($loc_name(2) != "")
> +		&& ($loc_name(3) != "")} {
> +		pass $gdb_test_name
> +	    } else {
> +		fail $gdb_test_name
> +	    }

Instead of

  if { something } {
    pass foo
  } else {
    fail foo
  }

You can use the gdb_assert proc:

  gdb_assert something foo

And to make it easier to narrow down the issue if that fails, I'd make
separate calls to gdb_assert:

    gdb_assert { ![string equal $loc_name(1) $loc_name(2)] }
    gdb_assert { ![string equal $loc_name(1) $loc_name(3)] }
    gdb_assert { ![string equal $loc_name(2) $loc_name(3)] }
    gdb_assert { [string length $loc_name(1)] > 0 }
    gdb_assert { [string length $loc_name(2)] > 0 }
    gdb_assert { [string length $loc_name(3)] > 0 }

Comparing strings with != would make quoting particularly bad in this
case, that's why I used string equal instead.

> +	}
> +    }
> +}
>
> -proc test_info_break {suffix} {
> -    global bpnum1 bpnum2 fill
> +# Test the breakpoint location enabled states.  STATES is a list of
> +# location states.  We assume STATES to contain the state for A, Base,
> +# and C, and in this order.  E.g. {N* y n} for 'N*' at A::func, 'y' at
> +# B::func, and 'n' at C::func, respectively.
>
> -    set bp_hit_info "${fill}(\r\n${fill}breakpoint already hit 1 time)?"
> +proc check_bp_locations {bpnum states cond {msg ""}} {
> +    global loc_name fill bpnum1 bpnum2

bpnum1 and bpnum2 are not used in this proc.

Otherwise, LGTM, thanks!

Simon