Re: [PATCH][gdb/testsuite] Don't abort testrun for invalid command in test-case

[Pedro Alves <palves@redhat.com>]

On 6/11/20 5:25 PM, Tom de Vries wrote:
> On 11-06-2020 17:31, Pedro Alves wrote:

>> I'd think it would be cleaner to override unknown in gdb_init,
>> and restore in gdb_finish.  No need for filename matching that way.
>> Like below.  Any reason you didn't go for this instead?
>>
> I don't think it's cleaner, because we run default_gdb_init, as well the
> bit of proc runtest between ${tool}_init a ${tool}_finish with the
> altered ::unknown.

Well, that is code that is exercised by all testscases, it's part of the
framework.  It's not going to ever contain calls to procedures that
don't exist that go unnoticed for long.  The whole testsuite would
collapse.  We can move the overriding until after default_gdb_init
is called, even, so that even less code runs under gdb's "unknown":

    set res [default_gdb_init $test_file_name]

    # Skip dejagnu_unknown while running a testcase to prevent it from
    # exiting and aborting the entire test run.  Save the unknown proc
    # defined by DejaGnu, and override the unknown proc with a
    # gdb-local version.
    rename ::unknown ::dejagnu_unknown
    proc unknown { args } {
	return [uplevel 1 ::tcl_unknown $args]
    }

    return $res
  }

> Alternatively, we could override source to detect the precise point that
> runtest hands control to the test-case, as attached.  But there's still
> filename matching.  [ Note btw, that this approach changes the scope of
> the fix slightly. ]

Overriding standard procs and filename matching strikes me as uglier than
using the hooks that dejagnu provides (init/finish), but I can also see your
point of only overriding unknown for the exact duration of the sourced file
being cleaner.  Thus, I'm not objecting and leave it up to you.  I agree
that it's nice that we don't abort the whole testsuite.

BTW, isn't "tcl_unknown" an internal DejaGnu name?  We should document
that in a comment, since this could break if DejaGnu changes the renamed
proc's name.  Another reason to ask DejaGnu to handle this for us cleanly.

Thanks,
Pedro Alves