dejagnu
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/2] Move loaded_libs check into search_and_load_file


From: Andrew Burgess
Subject: [PATCH 2/2] Move loaded_libs check into search_and_load_file
Date: Thu, 7 Mar 2019 14:01:57 +0000

Some users of search_and_load_file want to load the file only if it
has not been loaded before, while others want to always load the
file (this is might be because "that's what they've always done"
rather than the design, but I'm not sure about that).

However, changes to load_tool_init in this commit:

  commit c0310e95ba22fe548fe270b4af51065aceb17dcb
  Date:   Thu Nov 15 18:21:18 2018 +1100

broke GDB testing.  The reason is that GDB stores its tool init file
as testsuite/lib/gdb.exp, and then _incorrectly_ calls 'load_lib
gdb.exp' from some of the target init files.

The result of the above commit is that gdb.exp is now loaded twice.

It wouldn't be so bad if DeJaGNU always spotted this as an error and
complained, however, this multiple inclusion will run quite fine until
some seemingly harmless change to gdb.exp means the file can now only
be parsed once, at which point the user sees an non-obvious error.

One possible solution would be to make search_and_load_file load any
file only once, however, some users of search_and_load_file have,
historically, allowed files to be loaded multiple times, and breaking
that might impact existing users of DeJaGNU.

The solution presented here has search_and_load_file take a callback
function, this allows the "has this file been loaded already?" check
to only be performed for the users that care, while also allowing the
check to be deferred until we know exactly which file we are going to
load.  I also include a check specifically to spot the case where a
tool init file is loaded as a library, and warn about it.

ChangeLog:

        * runtest.exp (search_and_load_file): Add filter_func argument,
        use this to filter files before loading.
        (filter_load_always): New proc.
        (filter_load_once): New proc.
        (load_lib): Remove loaded_libs check, pass filter to
        search_and_load_file.
        (load_tool_init): Likewise.
        (load_generic_config): Pass filter to search_and_load_file.
        (load_tool_target_config): Likewise.
        (load_board_description): Likewise.
        (load_base_board_description): Likewise.
---
 ChangeLog   | 14 ++++++++++++
 runtest.exp | 72 ++++++++++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/runtest.exp b/runtest.exp
index c4dd207..d46ab7f 100644
--- a/runtest.exp
+++ b/runtest.exp
@@ -322,8 +322,10 @@ proc load_file { args } {
 #
 # search_and_load_file -- search DIRLIST looking for FILELIST.
 # TYPE is used when displaying error and progress messages.
+# FILTER_FUNC is a callback to decide if the file should be
+# loaded or not.
 #
-proc search_and_load_file { type filelist dirlist } {
+proc search_and_load_file { type filelist dirlist filter_func } {
     set found 0
 
     foreach dir $dirlist {
@@ -333,6 +335,15 @@ proc search_and_load_file { type filelist dirlist } {
            if {[file exists $filename]} {
                set found 1
                set error ""
+
+               if { ![$filter_func $filename $type] } {
+                    # Return true to indicate the file was found, even
+                    # though we didn't load it.  Callers to
+                    # search_and_load_file treat a false return value
+                    # as indicating a file not found error.
+                   return $found
+               }
+
                if { $type ne "library file" } {
                    send_user "Using $filename as $type.\n"
                } else {
@@ -356,6 +367,36 @@ proc search_and_load_file { type filelist dirlist } {
     return $found
 }
 
+#
+# Used by search_and_load_file as a filter function to always
+# load the found file.
+#
+proc filter_load_always { filename type } {
+    verbose "filter_load_always: $filename" 2
+    return 1
+}
+
+#
+# Used by search_and_load_file as a filter function, only load
+# the file if it has not been loaded before.
+#
+proc filter_load_once { filename type } {
+    global loaded_libs
+
+    if {[info exists loaded_libs($filename)]} {
+        if { $loaded_libs($filename) == "tool init file"
+                 && $type == "library file" } {
+            send_error "WARNING: Tool init file $filename also being loaded as 
a library.\n"
+        }
+        verbose "filter_load_once: $filename (already loaded)" 2
+       return 0
+    }
+
+    verbose "filter_load_once: $filename (loadeding)" 2
+    set loaded_libs($filename) $type
+    return 1
+}
+
 #
 # Give a usage statement.
 #
@@ -599,13 +640,7 @@ proc lookfor_file { dir name } {
 proc load_lib { file } {
     global verbose execpath tool
     global libdir libdirs srcdir testsuitedir base_dir
-    global loaded_libs
 
-    if {[info exists loaded_libs($file)]} {
-       return
-    }
-
-    set loaded_libs($file) ""
     set search_dirs [list ../lib $libdir $libdir/lib]
     lappend search_dirs [file dirname [file dirname $srcdir]]/dejagnu/lib
     lappend search_dirs $testsuitedir/lib
@@ -614,7 +649,7 @@ proc load_lib { file } {
     if {[info exists libdirs]} {
        lappend search_dirs $libdirs
     }
-    if { [search_and_load_file "library file" $file $search_dirs ] == 0 } {
+    if { [search_and_load_file "library file" $file $search_dirs 
"filter_load_once"] == 0 } {
        send_error "ERROR: Couldn't find library file $file.\n"
        exit 1
     }
@@ -983,13 +1018,6 @@ if { $target_os eq "" } {
 
 proc load_tool_init { file } {
     global srcdir testsuitedir
-    global loaded_libs
-
-    if {[info exists loaded_libs(tool/$file)]} {
-       return
-    }
-
-    set loaded_libs(tool/$file) ""
 
     lappend searchpath [file join $testsuitedir lib tool]
     lappend searchpath [file join $testsuitedir lib]
@@ -997,7 +1025,7 @@ proc load_tool_init { file } {
     # testsuite/lib/ in the package source tree; deprecated
     lappend searchpath [file join $srcdir lib]
 
-    if { ![search_and_load_file "tool init file" [list $file] $searchpath] } {
+    if { ![search_and_load_file "tool init file" [list $file] $searchpath 
"filter_load_once"] } {
        warning "Couldn't find tool init file"
     }
 }
@@ -1322,7 +1350,7 @@ proc load_generic_config { name } {
     }
 
     set dirlist [concat $libdir/config [file dirname $libdir]/config 
$boards_dir]
-    set result [search_and_load_file "generic interface file $type" $name.exp 
$dirlist]
+    set result [search_and_load_file "generic interface file $type" $name.exp 
$dirlist "filter_load_always"]
 
     return $result
 }
@@ -1335,7 +1363,7 @@ proc load_config { args } {
 
     set found 0
 
-    return [search_and_load_file "tool-and-target-specific interface file" 
$args [list $testsuitedir/config $testsuitedir/../config 
$testsuitedir/../../config $testsuitedir/../../../config]]
+    return [search_and_load_file "tool-and-target-specific interface file" 
$args [list $testsuitedir/config $testsuitedir/../config 
$testsuitedir/../../config $testsuitedir/../../../config] "filter_load_always"]
 }
 
 #
@@ -1363,7 +1391,7 @@ proc load_tool_target_config { name } {
        send_error "WARNING: Couldn't find tool config file for $name, using 
default.\n"
        # If we can't load the tool init file, this must be a simple natively 
hosted
        # test suite, so we use the default procs for Unix.
-       if { [search_and_load_file "library file" default.exp [list $libdir 
$libdir/config [file dirname [file dirname $testsuitedir]]/dejagnu/config 
$testsuitedir/config . [file dirname [file dirname [file dirname 
$testsuitedir]]]/dejagnu/config]] == 0 } {
+       if { [search_and_load_file "library file" default.exp [list $libdir 
$libdir/config [file dirname [file dirname $testsuitedir]]/dejagnu/config 
$testsuitedir/config . [file dirname [file dirname [file dirname 
$testsuitedir]]]/dejagnu/config] "filter_load_always"] == 0 } {
            send_error "ERROR: Couldn't find default tool init file.\n"
            exit 1
        }
@@ -1426,8 +1454,8 @@ proc load_board_description { board_name args } {
            set board_info($whole_name,isremote) 0
        }
     }
-    search_and_load_file "standard board description file $type" standard.exp 
$dirlist
-    set found [search_and_load_file "board description file $type" 
$board_name.exp $dirlist]
+    search_and_load_file "standard board description file $type" standard.exp 
$dirlist "filter_load_always"
+    set found [search_and_load_file "board description file $type" 
$board_name.exp $dirlist "filter_load_always"]
     if { $board_set != 0 } {
        unset board
     }
@@ -1469,7 +1497,7 @@ proc load_base_board_description { board_name } {
     if { $board_name eq [get_local_hostname] } {
        set board_info($board_name,isremote) 0
     }
-    set found [search_and_load_file "board description file $type" 
$board_name.exp [list $libdir/baseboards]]
+    set found [search_and_load_file "board description file $type" 
$board_name.exp [list $libdir/baseboards] "filter_load_always"]
     if { $board_set != 0 } {
        unset board
     }
-- 
2.14.5




reply via email to

[Prev in Thread] Current Thread [Next in Thread]