diff --git a/test/SIM_mtv/RUN_test/input.py b/test/SIM_mtv/RUN_test/input.py index 70f0896f2..252afe8e4 100644 --- a/test/SIM_mtv/RUN_test/input.py +++ b/test/SIM_mtv/RUN_test/input.py @@ -31,13 +31,21 @@ def main(): trick.add_event(event_a) # ------------------------------------------------------- - # Event B: condition_var + action so the test can verify - # a second event appears in mtv_count and that - # condition_list / action_list paths work for index 1 too. + # Event B: lives at mtv_list index 1, with TWO conditions + # and TWO actions so condition_list[1] / action_list[1] + # exist. Index >= 1 is the regression case from issue + # #2139: MemoryManager's dynamic-array bounds check + # rejected mtv_list[1] (and any other index >= 1 on a + # named TMM pointer array), so every event after the + # first came back as BAD_REF. + # condition_var() gets no comment, so its comment + # defaults to the variable name string. # ------------------------------------------------------- event_b = trick.new_event("event_b") event_b.condition_var(0, "mobj.fire_condition") - event_b.action(0, "mobj.action_counter+=10", "act_b_comment") + event_b.condition(1, "False", "cond_b_comment_1") + event_b.action(0, "mobj.action_counter+=10", "act_b_comment_0") + event_b.action(1, "mobj.action_counter+=100", "act_b_comment_1") event_b.set_cycle(0.1) event_b.activate() trick.add_event(event_b) diff --git a/test/SIM_mtv/models/test_client/test_client.cpp b/test/SIM_mtv/models/test_client/test_client.cpp index 23ead2496..8385bcd02 100644 --- a/test/SIM_mtv/models/test_client/test_client.cpp +++ b/test/SIM_mtv/models/test_client/test_client.cpp @@ -1,9 +1,27 @@ /* * MTV variable-server path tests. * - * These tests verify the bug fix where MTV used "cond[N]" and "act[N]" as - * variable server paths instead of the correct "condition_list[N]" and - * "action_list[N]" field names. + * These tests verify two classes of bug fixes: + * + * 1. Issue #2020 / PR #2092: MTV used "cond[N]" and "act[N]" as variable + * server paths instead of the correct "condition_list[N][0]" and + * "action_list[N][0]" field names. + * + * 2. Issue #2139 / PR #2141: events with index N>=1 failed to load. + * MemoryManager_ref_dim.cpp double-dereferenced the address in its + * dynamic-array bounds check, so "mtv_list[1]" (or any index >= 1 on a + * named TMM pointer array) was rejected as out of bounds and every path + * under it resolved to BAD_REF. The original tests missed this because + * they only asserted on event index 0 values, and a bad reference is + * still added and sent (as the string "BAD_REF"), so token/size counts + * look normal. Lesson: assert on *values* at index >= 1, and scan for + * BAD_REF; counting tokens proves nothing. + * + * The variable paths used here must stay in sync with the paths MtvApp.java + * builds (see "get cyclical event data" in MtvApp.java and + * Trick::MTV::send_event_data() in MTV.cpp). #2139 happened partly because + * these tests hand-wrote correct paths while MtvApp.java emitted broken + * ones — if you change the paths in either place, change them here too. * * Protocol note (ASCII mode): * var_send_once("x") -> "5\t\n" @@ -49,6 +67,55 @@ static std::string get_once(Socket& s, const std::string& varpath) return tokens[1]; } +// Build the var_add commands MtvApp.java sends for one event's cyclic status +// variables, and bump var_count by the number of variables added. +// MUST stay in sync with the "get cyclical event data" section of +// MtvApp.java — replicating MtvApp's real subscription set is the point. +static std::string mtvapp_event_vars(int ii, int cond_count, int act_count, int& var_count) +{ + std::ostringstream cmd; + std::string event = "trick_ip.mtv.mtv_list[" + std::to_string(ii) + "][0]"; + + const char* event_fields[] + = { "active", "fired_time", "fired_count", "ran_time", "ran_count", "manual", "manual_fired", "hold", "added" }; + for (const char* field : event_fields) + { + cmd << "trick.var_add(\"" << event << "." << field << "\"); "; + var_count++; + } + const char* cond_fields[] = { "enabled", "fired_time", "fired_count", "hold" }; + for (int jj = 0; jj < cond_count; jj++) + { + for (const char* field : cond_fields) + { + cmd << "trick.var_add(\"" << event << ".condition_list[" << jj << "][0]." << field << "\"); "; + var_count++; + } + } + const char* act_fields[] = { "enabled", "ran_time", "ran_count" }; + for (int jj = 0; jj < act_count; jj++) + { + for (const char* field : act_fields) + { + cmd << "trick.var_add(\"" << event << ".action_list[" << jj << "][0]." << field << "\"); "; + var_count++; + } + } + return cmd.str(); +} + +// Fail the calling test if any token in a var server reply is BAD_REF. +// An unresolvable path is still added and sent as the literal string +// "BAD_REF", so counting tokens can never detect a broken path. +static void expect_no_bad_ref(const std::vector& tokens, const std::string& context) +{ + for (size_t i = 0; i < tokens.size(); i++) + { + EXPECT_NE(tokens[i], "BAD_REF") << context << ": token " << i + << " is BAD_REF — a variable path did not resolve"; + } +} + // --------------------------------------------------------------------------- // Test 1 – EventCount // @@ -66,10 +133,10 @@ TEST_F(MtvTest, EventCount) // --------------------------------------------------------------------------- // Test 2 – SendEventDataComments // -// Call trick.mtv_send_event_data() and verify: -// a) The list-size message arrives with the right variable count. -// b) condition_list[0].comment == "cond_comment_0" (was broken pre-fix) -// c) action_list[0].comment == "act_comment_0" (was broken pre-fix) +// Call trick.mtv_send_event_data() and verify the full data block for BOTH +// events. Asserting on event_b (mtv_list index 1) is what catches the #2139 +// MemoryManager bounds-check regression: pre-fix, every mtv_list[1][0].x +// path resolved to BAD_REF while the variable count stayed correct. // // For event_a (1 condition, 1 action, before_after=0): // base fields: name, active, added, condition_count, action_count, @@ -78,10 +145,13 @@ TEST_F(MtvTest, EventCount) // + act comments: + 1 // subtotal event_a: = 8 // -// For event_b (1 condition, 1 action, before_after=0): -// same layout: = 8 +// For event_b (2 conditions, 2 actions, before_after=0): +// base fields: = 6 +// + cond comments: + 2 +// + act comments: + 2 +// subtotal event_b: = 10 // -// Total variables: = 16 +// Total variables: = 18 // --------------------------------------------------------------------------- TEST_F(MtvTest, SendEventDataComments) { @@ -97,23 +167,24 @@ TEST_F(MtvTest, SendEventDataComments) EXPECT_EQ(size_tokens[0], "3") << "Expected VS_LIST_SIZE message type (3)"; int var_count = std::stoi(size_tokens[1]); - EXPECT_EQ(var_count, 16) << "Expected 16 variables for 2 events (8 each)"; + EXPECT_EQ(var_count, 18) << "Expected 18 variables for 2 events (8 + 10)"; // Second reply: the data "0\t\t...\n" std::string data_line = socket.recv_line(); auto data = split_tabs(data_line); - ASSERT_GE(data.size(), 17u) // message-type token + 16 values + ASSERT_GE(data.size(), 19u) // message-type token + 18 values << "Not enough tokens in data reply. Got: " << data_line; + expect_no_bad_ref(data, "mtv_send_event_data reply"); + // data[0] = message type "0" - // data[1] = event_a.name - // data[2] = event_a.active - // data[3] = event_a.added - // data[4] = event_a.condition_count - // data[5] = event_a.action_count - // data[6] = event_a.before_after - // data[7] = condition_list[0][0].comment <- was broken pre-fix - // data[8] = action_list[0][0].comment <- was broken pre-fix + // + // event_a block (mtv_list[0][0]): + // data[1] = name, data[2] = active, data[3] = added, + // data[4] = condition_count, data[5] = action_count, + // data[6] = before_after, + // data[7] = condition_list[0][0].comment <- broken pre-#2092 + // data[8] = action_list[0][0].comment <- broken pre-#2092 EXPECT_EQ(data[1], "event_a"); EXPECT_EQ(data[4], "1") << "event_a should have 1 condition"; EXPECT_EQ(data[5], "1") << "event_a should have 1 action"; @@ -121,41 +192,61 @@ TEST_F(MtvTest, SendEventDataComments) << "condition_list[0][0].comment mismatch – was the cond->condition_list[N][0] fix applied?"; EXPECT_EQ(data[8], "act_comment_0") << "action_list[0][0].comment mismatch – was the act->action_list[N][0] fix applied?"; + + // event_b block (mtv_list[1][0]) — every value here was BAD_REF pre-#2141: + // data[9] = name, data[10] = active, data[11] = added, + // data[12] = condition_count, data[13] = action_count, + // data[14] = before_after, + // data[15] = condition_list[0][0].comment (condition_var: defaults to var name) + // data[16] = condition_list[1][0].comment + // data[17] = action_list[0][0].comment + // data[18] = action_list[1][0].comment + EXPECT_EQ(data[9], "event_b") << "mtv_list[1][0].name failed to resolve – #2139 regression?"; + EXPECT_EQ(data[12], "2") << "event_b should have 2 conditions"; + EXPECT_EQ(data[13], "2") << "event_b should have 2 actions"; + EXPECT_EQ(data[15], "mobj.fire_condition"); + EXPECT_EQ(data[16], "cond_b_comment_1") << "condition_list[1][0].comment failed – condition index >= 1 broken?"; + EXPECT_EQ(data[17], "act_b_comment_0"); + EXPECT_EQ(data[18], "act_b_comment_1") << "action_list[1][0].comment failed – action index >= 1 broken?"; } // --------------------------------------------------------------------------- // Test 3 – CyclicConditionList // -// Subscribe cyclically to condition_list[0] fields on event_a. -// Before the fix, the var server would reject these paths because the field -// was still named "cond" internally. Successful subscription (non-empty -// numeric reply) proves the path resolves correctly. +// Subscribe cyclically to condition_list fields at index 0 (event_a) AND at +// event index 1 / condition index 1 (event_b's second condition). The +// index >= 1 paths are the #2139 regression case: pre-fix they resolved to +// BAD_REF, while index 0 worked. // --------------------------------------------------------------------------- TEST_F(MtvTest, CyclicConditionList) { ASSERT_EQ(status, 0) << "Could not connect to variable server"; // Subscribe to all four condition_list fields MtvApp.java uses. - socket << "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].enabled\")\n" - "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].fired_time\")\n" - "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].fired_count\")\n" - "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].hold\")\n" - "trick.var_cycle(0.1)\n"; - - // Read one cycle of data; the var server should send back 4 values. + socket << "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].enabled\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].fired_time\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].fired_count\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].condition_list[0][0].hold\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].condition_list[1][0].enabled\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].condition_list[1][0].fired_time\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].condition_list[1][0].fired_count\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].condition_list[1][0].hold\")\n" + "trick.var_cycle(0.1)\n"; + + // Read one cycle of data; the var server should send back 8 values. std::string reply = socket.recv_line(); auto tokens = split_tabs(reply); - // Message type 0 + 4 value tokens = 5 tokens minimum. - ASSERT_GE(tokens.size(), 5u) - << "condition_list subscription returned too few tokens.\n" - << "Reply: " << reply << "\n" - << "This indicates the variable path was not resolved (pre-fix 'cond' names)."; + // Message type 0 + 8 value tokens = 9 tokens minimum. + ASSERT_GE(tokens.size(), 9u) << "condition_list subscription returned too few tokens.\n" + << "Reply: " << reply; EXPECT_EQ(tokens[0], "0") << "Expected var_send data message type (0)"; + expect_no_bad_ref(tokens, "condition_list cyclic reply"); // enabled defaults to true (1) for a freshly created condition. - EXPECT_EQ(tokens[1], "1") << "condition_list[0].enabled should be 1 (true)"; + EXPECT_EQ(tokens[1], "1") << "condition_list[0][0].enabled should be 1 (true)"; + EXPECT_EQ(tokens[5], "1") << "mtv_list[1][0].condition_list[1][0].enabled should be 1 – index >= 1 broken (#2139)?"; socket << "trick.var_clear()\n"; } @@ -163,34 +254,73 @@ TEST_F(MtvTest, CyclicConditionList) // --------------------------------------------------------------------------- // Test 4 – CyclicActionList // -// Same as Test 3 but for action_list[0] fields. +// Same as Test 3 but for action_list fields. // --------------------------------------------------------------------------- TEST_F(MtvTest, CyclicActionList) { ASSERT_EQ(status, 0) << "Could not connect to variable server"; socket << "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].action_list[0][0].enabled\")\n" - "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].action_list[0][0].ran_time\")\n" - "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].action_list[0][0].ran_count\")\n" - "trick.var_cycle(0.1)\n"; + "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].action_list[0][0].ran_time\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[0][0].action_list[0][0].ran_count\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].action_list[1][0].enabled\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].action_list[1][0].ran_time\")\n" + "trick.var_add(\"trick_ip.mtv.mtv_list[1][0].action_list[1][0].ran_count\")\n" + "trick.var_cycle(0.1)\n"; + + std::string reply = socket.recv_line(); + auto tokens = split_tabs(reply); + + // Message type 0 + 6 value tokens = 7 tokens minimum. + ASSERT_GE(tokens.size(), 7u) << "action_list subscription returned too few tokens.\n" + << "Reply: " << reply; + + EXPECT_EQ(tokens[0], "0") << "Expected var_send data message type (0)"; + expect_no_bad_ref(tokens, "action_list cyclic reply"); + + EXPECT_EQ(tokens[1], "1") << "action_list[0][0].enabled should be 1 (true)"; + EXPECT_EQ(tokens[4], "1") << "mtv_list[1][0].action_list[1][0].enabled should be 1 – index >= 1 broken (#2139)?"; + + socket << "trick.var_clear()\n"; +} + +// --------------------------------------------------------------------------- +// Test 5 – MtvAppCyclicVarSet +// +// Replicate the exact cyclic subscription MtvApp.java builds after loading +// the event list (time vars + per-event status vars for both events), then +// verify every variable resolves to a real value. This is what MTV actually +// does when it connects, so a path MtvApp constructs that the variable +// server cannot resolve fails here instead of only failing when a user +// connects the real GUI. +// --------------------------------------------------------------------------- +TEST_F(MtvTest, MtvAppCyclicVarSet) +{ + ASSERT_EQ(status, 0) << "Could not connect to variable server"; + + int var_count = 0; + std::string cmd = "trick.var_add(\"trick_sys.sched.time_tics\"); " + "trick.var_add(\"trick_ip.mtv.mtv_update_ticker\"); "; + var_count += 2; + cmd += mtvapp_event_vars(0, 1, 1, var_count); // event_a: 1 condition, 1 action + cmd += mtvapp_event_vars(1, 2, 2, var_count); // event_b: 2 conditions, 2 actions + socket << cmd + "\n" + "trick.var_cycle(0.1)\n"; std::string reply = socket.recv_line(); auto tokens = split_tabs(reply); - // Message type 0 + 3 value tokens = 4 tokens minimum. - ASSERT_GE(tokens.size(), 4u) - << "action_list subscription returned too few tokens.\n" - << "Reply: " << reply << "\n" - << "This indicates the variable path was not resolved (pre-fix 'act' names)."; + ASSERT_EQ(tokens.size(), (size_t)var_count + 1) // message-type token + values + << "MtvApp cyclic subscription returned wrong token count.\n" + << "Reply: " << reply; EXPECT_EQ(tokens[0], "0") << "Expected var_send data message type (0)"; - EXPECT_EQ(tokens[1], "1") << "action_list[0].enabled should be 1 (true)"; + expect_no_bad_ref(tokens, "MtvApp cyclic reply"); socket << "trick.var_clear()\n"; } // --------------------------------------------------------------------------- -// Test 5 – UpdateTicker +// Test 6 – UpdateTicker // // Verify mtv_update_ticker increments when an event is deleted. // This confirms the MTV bookkeeping that drives MtvApp's re-sync logic. @@ -212,8 +342,7 @@ TEST_F(MtvTest, UpdateTicker) std::string after = get_once(socket, "trick_ip.mtv.mtv_update_ticker"); int ticker_after = std::stoi(after); - EXPECT_GT(ticker_after, ticker_before) - << "mtv_update_ticker should have incremented after delete_event()"; + EXPECT_GT(ticker_after, ticker_before) << "mtv_update_ticker should have incremented after delete_event()"; } // --------------------------------------------------------------------------- diff --git a/trick_source/java/src/main/java/trick/mtv/MtvApp.java b/trick_source/java/src/main/java/trick/mtv/MtvApp.java index 4d1226e21..c843be20c 100644 --- a/trick_source/java/src/main/java/trick/mtv/MtvApp.java +++ b/trick_source/java/src/main/java/trick/mtv/MtvApp.java @@ -40,7 +40,7 @@ public class MtvApp extends TrickApplication { public double sim_time = 0.0; // 1st var to get: Current sim time public static int last_update_ticker; // 2nd var to get: Running count of mtv table added or deleted event(s) public final int time_tics = 1000000; // For converting time tics to seconds - //private boolean updating=false; // True when mtv gui is being updated, so ignore var server data + // private boolean updating=false; // True when mtv gui is being updated, so ignore var server data public static boolean confirmExit = true; // Do you want the "do you really want to exit" popup ?? public boolean need_update = false; // True when event data has changed and need to udpate display public boolean one_shot = false; // True when paused and need to get one shot of data from var server @@ -67,7 +67,8 @@ public class EventInfo { JRadioButton butt; // button used in customize_event_display } - public static ArrayList mtv_events = new ArrayList(100); // Save info about events in gui (mtv_count) + public static ArrayList mtv_events = + new ArrayList(100); // Save info about events in gui (mtv_count) /** * Main method launching the application. @@ -121,7 +122,7 @@ protected void initialize(String[] args) { JProgressBar bar; boolean max_reached = false; // only allow max_events_show number of events in table - //System.out.println("Initialize."); + // System.out.println("Initialize."); // Connect to variable server and request event info try { if (vscom == null) { @@ -134,18 +135,18 @@ protected void initialize(String[] args) { // already connected, so we must be updating because event data has changed } if (vscom != null) { - //System.out.println("Connect OK."); + // System.out.println("Connect OK."); vscom.pause(); vscom.put("trick.var_set_client_tag(\"TRICK_MTV\")"); - //vscom.setAscii(); - //vscom.setBinary(); + // vscom.setAscii(); + // vscom.setBinary(); vscom.setBinaryNoNames(); // -------------------------- get preliminary event data --------------------------- - //vscom.put("trick.var_debug(3)") ; + // vscom.put("trick.var_debug(3)") ; vscom.put("trick.var_cycle(0.01)"); // get preliminary event data at this rate vscom.put("trick.var_add(\"trick_ip.mtv.mtv_count\"); trick.var_send(); trick.var_clear();"); mtv_list_count = Integer.parseInt(vscom.get()); - //System.out.println("mtv_count=" + mtv_list_count); + // System.out.println("mtv_count=" + mtv_list_count); // create a popup progress bar to show while getting event data popup = new JFrame(); @@ -165,6 +166,10 @@ protected void initialize(String[] args) { popup.setSize(400, 100); popup.setVisible(true); + // NOTE: The variable order parsed below is produced by Trick::MTV::send_event_data() + // (MTV.cpp) and is verified by test/SIM_mtv (SendEventDataComments in + // test_client.cpp). If the layout changes in any of these places, the + // others must change with it. vscom.put("trick.mtv_send_event_data();"); // get all event data from Trick numvars = Integer.parseInt(vscom.get()); results = vscom.get(numvars).split("\t"); @@ -173,13 +178,14 @@ protected void initialize(String[] args) { end = mtv_list_count; mtv_count = 0; // how many events are shown for (int ii = start; ii < end; ii++) { - // get each event name, active, added, number of conditions, number of actions, before/after indicator + // get each event name, active, added, number of conditions, number of actions, before/after + // indicator if (ii == mtv_events.size()) { // new event data since previous load mtv_events.add(ii, new EventInfo()); mtv_events.get(ii).show = (Integer.parseInt(results[rindex + 2]) == 1); // show it if it's added mtv_events.get(ii).deleted = false; - mtv_events.get(ii).butt = new JRadioButton(results[rindex + 0]); //name + mtv_events.get(ii).butt = new JRadioButton(results[rindex + 0]); // name } mtv_events.get(ii).name = results[rindex + 0]; mtv_events.get(ii).active = (Integer.parseInt(results[rindex + 1]) == 1); @@ -197,7 +203,8 @@ protected void initialize(String[] args) { popup.repaint(); popup.update(popup.getGraphics()); - // read events have no name, or user may not have specified an event name, default is do not show these + // read events have no name, or user may not have specified an event name, default is do not show + // these if (mtv_events.get(ii).name.equals("no_name_specified") && (mtv_view == null)) { mtv_events.get(ii).show = false; } @@ -221,21 +228,24 @@ protected void initialize(String[] args) { mtv_events.get(ii).cond = new String[mtv_events.get(ii).cond_count]; for (int jj = 0; jj < mtv_events.get(ii).cond_count; jj++) { - //vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].cond[" +jj+ "].comment\"); trick.var_send(); trick.var_clear();"); - //mtv_events.get(ii).cond[jj] = vscom.get(); + // vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].cond[" +jj+ "].comment\"); + // trick.var_send(); trick.var_clear();"); + // mtv_events.get(ii).cond[jj] = vscom.get(); mtv_events.get(ii).cond[jj] = results[rindex + jj]; } rindex += mtv_events.get(ii).cond_count; mtv_events.get(ii).act = new String[mtv_events.get(ii).act_count]; for (int jj = 0; jj < mtv_events.get(ii).act_count; jj++) { - //vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].act[" +jj+ "].comment\"); trick.var_send(); trick.var_clear();"); - //mtv_events.get(ii).act[jj] = vscom.get(); + // vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].act[" +jj+ "].comment\"); + // trick.var_send(); trick.var_clear();"); + // mtv_events.get(ii).act[jj] = vscom.get(); mtv_events.get(ii).act[jj] = results[rindex + jj]; } rindex += mtv_events.get(ii).act_count; if (mtv_events.get(ii).before_after > 0) { - //vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].target_job\"); trick.var_send(); trick.var_clear();"); - //mtv_events.get(ii).target_job = vscom.get(); + // vscom.put("trick.var_add(\"trick_ip.mtv.mtv_list[" +ii+ "][0].target_job\"); + // trick.var_send(); trick.var_clear();"); + // mtv_events.get(ii).target_job = vscom.get(); mtv_events.get(ii).target_job = results[rindex + 0]; rindex++; } @@ -247,12 +257,19 @@ protected void initialize(String[] args) { map = new int[mtv_count]; // this will map the gui event table index to the mtv_events data // -------------------------- get cyclical event data --------------------------- + // NOTE: The variable paths built below MUST stay in step with the SIM_mtv test + // (mtvapp_event_vars() in test/SIM_mtv/models/test_client/test_client.cpp), + // which replicates this exact subscription set so CI catches paths the + // variable server cannot resolve. Issue #2139 (events with index >= 1 + // failing to load) slipped through because the test hand-wrote correct + // paths while this code emitted broken ones. If you add, remove, or + // rename a var_add below, update the test to match. mtv_var_count = 0; vscom.put("trick.var_cycle(" + var_cycle + ")"); // tell var server to get sim time and mtv_udpate_ticker cyclically time_vars = "trick.var_add(\"trick_sys.sched.time_tics\"); "; time_vars += "trick.var_add(\"trick_ip.mtv.mtv_update_ticker\"); "; - //System.out.println(time_vars); + // System.out.println(time_vars); vscom.put(time_vars); mtv_var_count += 2; @@ -263,44 +280,54 @@ protected void initialize(String[] args) { } // tell var server to get status info for each event that we will read cyclically map[mtv_count] = ii; - event_vars = "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].active\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].fired_time\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].fired_count\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].ran_time\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].ran_count\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].manual\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].manual_fired\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].hold\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].added\"); "; + event_vars = String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].active\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].fired_time\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].fired_count\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].ran_time\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].ran_count\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].manual\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].manual_fired\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].hold\"); ", ii); + event_vars += String.format("trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].added\"); ", ii); mtv_var_count += 9; for (int jj = 0; jj < mtv_events.get(ii).cond_count; jj++) { - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].condition_list[" + jj + "].enabled\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].condition_list[" + jj + "].fired_time\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].condition_list[" + jj + "].fired_count\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].condition_list[" + jj + "].hold\"); "; + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].condition_list[%d][0].enabled\"); ", + ii, jj); + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].condition_list[%d][0].fired_time\"); ", + ii, jj); + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].condition_list[%d][0].fired_count\"); ", + ii, jj); + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].condition_list[%d][0].hold\"); ", ii, jj); mtv_var_count += 4; } for (int jj = 0; jj < mtv_events.get(ii).act_count; jj++) { - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].action_list[" + jj + "].enabled\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].action_list[" + jj + "].ran_time\"); "; - event_vars += "trick.var_add(\"trick_ip.mtv.mtv_list[" + ii + "][0].action_list[" + jj + "].ran_count\"); "; + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].action_list[%d][0].enabled\"); ", ii, jj); + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].action_list[%d][0].ran_time\"); ", + ii, jj); + event_vars += String.format( + "trick.var_add(\"trick_ip.mtv.mtv_list[%d][0].action_list[%d][0].ran_count\"); ", + ii, jj); mtv_var_count += 3; } - //System.out.println(event_vars); + // System.out.println(event_vars); vscom.put(event_vars); mtv_count++; } // end for ii - //System.out.println("mtv_var_count=" + mtv_var_count); + // System.out.println("mtv_var_count=" + mtv_var_count); if (max_reached) { JOptionPane.showMessageDialog( - popup, - "Number of events displayed is limited to a maximum of " + - max_events_show + - ".\n" + - "Use \"Customize Event Display\" under the View menu to display the ones you want.", - "Warning", - JOptionPane.WARNING_MESSAGE - ); + popup, + "Number of events displayed is limited to a maximum of " + max_events_show + + ".\n" + + "Use \"Customize Event Display\" under the View menu to display the ones you want.", + "Warning", + JOptionPane.WARNING_MESSAGE); } popup.dispose(); vscom.unpause(); @@ -319,7 +346,9 @@ public void add_my_exit_listener() { removeExitListener(exitListener); exitListener = new Application.ExitListener() { public boolean canExit(EventObject e) { - return (JOptionPane.showConfirmDialog(mtv_view.viewPanel, "Do you really want to exit?", "Confirm Exit", 0) == 0); + return (JOptionPane.showConfirmDialog( + mtv_view.viewPanel, "Do you really want to exit?", "Confirm Exit", 0) + == 0); } public void willExit(EventObject e) {} @@ -333,23 +362,28 @@ public void willExit(EventObject e) {} @Override protected void startup() { Vector row; - Vector default_row = new Vector(Arrays.asList(false, null, null, null, null, null, false, null, false)); - //Vector default_row = new Vector(Arrays.asList(false,null,null,null,null,null,false,null,null)); //DANNY add hidden column on end + Vector default_row = + new Vector(Arrays.asList(false, null, null, null, null, null, false, null, false)); + // Vector default_row = new + // Vector(Arrays.asList(false,null,null,null,null,null,false,null,null)); //DANNY add hidden column on + // end int rownum; boolean deadrow; - //System.out.println("Startup."); + // System.out.println("Startup."); if (mtv_view == null) { // set application icon ?? - getMainFrame().setIconImage(resourceMap.getImageIcon("Application.icon").getImage()); - confirmExit = Boolean.valueOf(MtvApp.getApplication().trickProperties.getProperty("confirmExit")); + getMainFrame() + .setIconImage(resourceMap.getImageIcon("Application.icon").getImage()); + confirmExit = + Boolean.valueOf(MtvApp.getApplication().trickProperties.getProperty("confirmExit")); if (confirmExit) { add_my_exit_listener(); } mtv_view = new MtvView(this); } if (!confirmExit) { - //remove exitlistener that super possibly added + // remove exitlistener that super possibly added removeExitListener(exitListener); } @@ -363,15 +397,15 @@ protected void startup() { for (int jj = 0; jj < mtv_events.get(map[ii]).cond_count; jj++) { row = new Vector(default_row); mtv_view.event_table_rows.addElement(row); - //rownum = mtv_view.event_table_rows.size()-1; - //mtv_view.event_table.setValueAt(" condition("+jj+")", rownum, 1); + // rownum = mtv_view.event_table_rows.size()-1; + // mtv_view.event_table.setValueAt(" condition("+jj+")", rownum, 1); } // Add a row for each action for (int jj = 0; jj < mtv_events.get(map[ii]).act_count; jj++) { row = new Vector(default_row); mtv_view.event_table_rows.addElement(row); - //rownum = mtv_view.event_table_rows.size()-1; - //mtv_view.event_table.setValueAt(" action("+jj+")", rownum, 1); + // rownum = mtv_view.event_table_rows.size()-1; + // mtv_view.event_table.setValueAt(" action("+jj+")", rownum, 1); } } @@ -386,7 +420,7 @@ protected void startup() { mtv_view.canEditRow = new boolean[mtv_view.event_table_rows.size()]; // set Active, Hold, and Mode checkboxes editable where appropriate mtv_view.canEdit = new boolean[mtv_view.event_table_rows.size()][mtv_view.event_table_cols.size()]; - //mtv_view.canEdit = new boolean[mtv_view.event_table_rows.size()][mtv_view.event_table_cols.size()+1]; //DANNY + // mtv_view.canEdit = new boolean[mtv_view.event_table_rows.size()][mtv_view.event_table_cols.size()+1]; //DANNY // keep track of when fired column is set to red & when ran column is set to green mtv_view.is_red = new boolean[mtv_view.event_table_rows.size()]; mtv_view.is_green = new boolean[mtv_view.event_table_rows.size()]; @@ -405,7 +439,7 @@ protected void startup() { deadrow = (mtv_events.get(map[ii]).name.equals("no_name_specified")); mtv_view.canEditRow[rownum] = !deadrow; mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 1); // Name - //mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY + // mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY for (int jj = 0; jj < mtv_events.get(map[ii]).cond_count; jj++) { rownum++; mtv_view.rowtype[rownum] = RowType.CONDITION; @@ -417,9 +451,10 @@ protected void startup() { mtv_view.canEdit[rownum][6] = true; // Hold mtv_view.canEdit[rownum][8] = false; // Added mtv_view.canEditRow[rownum] = !deadrow; - mtv_view.event_table.setValueAt("_cond(" + jj + "): " + mtv_events.get(map[ii]).cond[jj], rownum, 1); // Condition comment - //mtv_view.event_table.setValueAt(" cond("+jj+") ", rownum, 1); // Condition indicator - //mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY + mtv_view.event_table.setValueAt( + "_cond(" + jj + "): " + mtv_events.get(map[ii]).cond[jj], rownum, 1); // Condition comment + // mtv_view.event_table.setValueAt(" cond("+jj+") ", rownum, 1); // Condition indicator + // mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY } for (int jj = 0; jj < mtv_events.get(map[ii]).act_count; jj++) { rownum++; @@ -431,25 +466,26 @@ protected void startup() { mtv_view.canEdit[rownum][0] = true; // Active mtv_view.canEdit[rownum][8] = false; // Added mtv_view.canEditRow[rownum] = !deadrow; - mtv_view.event_table.setValueAt("__act(" + jj + "): " + mtv_events.get(map[ii]).act[jj], rownum, 1); // Action comment - //mtv_view.event_table.setValueAt(" action("+jj+")", rownum, 1); // Action indicator - //mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY + mtv_view.event_table.setValueAt( + "__act(" + jj + "): " + mtv_events.get(map[ii]).act[jj], rownum, 1); // Action comment + // mtv_view.event_table.setValueAt(" action("+jj+")", rownum, 1); // Action indicator + // mtv_view.event_table.setValueAt(mtv_events.get(map[ii]).name, rownum, 8); // Hidden Name DANNY } } /*** - //DANNY allow user to sort by columns - TableRowSorter sorter = - new TableRowSorter((DefaultTableModel) mtv_view.event_table.getModel()); - mtv_view.event_table.setRowSorter(sorter); - // sort by hidden event name column 8, then by name column 1 to keep each event's data together - List sortKeys = new ArrayList(); - sortKeys.add(new RowSorter.SortKey(8, SortOrder.ASCENDING)); - sortKeys.add(new RowSorter.SortKey(1, SortOrder.ASCENDING)); - sorter.setSortKeys(sortKeys); - ***/ + * //DANNY allow user to sort by columns + * TableRowSorter sorter = + * new TableRowSorter((DefaultTableModel) mtv_view.event_table.getModel()); + * mtv_view.event_table.setRowSorter(sorter); + * // sort by hidden event name column 8, then by name column 1 to keep each event's data together + * List sortKeys = new ArrayList(); + * sortKeys.add(new RowSorter.SortKey(8, SortOrder.ASCENDING)); + * sortKeys.add(new RowSorter.SortKey(1, SortOrder.ASCENDING)); + * sorter.setSortKeys(sortKeys); + ***/ // manually set the initial column sizes - //mtv_view.event_table.setAutoResizeMode(javax.swing.JTable.AUTO_RESIZE_OFF); + // mtv_view.event_table.setAutoResizeMode(javax.swing.JTable.AUTO_RESIZE_OFF); mtv_view.event_table.getColumnModel().getColumn(0).setPreferredWidth(20); // Active mtv_view.event_table.getColumnModel().getColumn(1).setPreferredWidth(225); // Name mtv_view.event_table.getColumnModel().getColumn(2).setPreferredWidth(72); // Fired Time @@ -484,28 +520,28 @@ protected void startup() { mtv_view.event_table.setEnabled(false); mtv_view.send_button.setEnabled(false); } - //updating = false; + // updating = false; } @Override protected void ready() { - //System.out.println("Ready."); + // System.out.println("Ready."); super.ready(); if (vscom == null) { return; } MonitorEventsTask monitorEventsTask = new MonitorEventsTask(this); - //monitorEventsTask.addPropertyChangeListener(this); + // monitorEventsTask.addPropertyChangeListener(this); getContext().getTaskService().execute(monitorEventsTask); } public void update() { // delete all rows from event table then redo it boolean background_reader_is_up = false; - //System.out.println("Update."); + // System.out.println("Update."); mtv_view.status_label.setText(" Updating..."); - //updating = true; + // updating = true; if (vscom != null) { background_reader_is_up = true; try { @@ -519,10 +555,11 @@ public void update() { try { // wait for a little more than a cycle so var server can finish sending any data long wait_msecs = (long) (var_cycle * 1000.0 + 50.0); - //System.out.println("...wait " + wait_msecs / 1000.0); + // System.out.println("...wait " + wait_msecs / 1000.0); Thread.sleep(wait_msecs); - //System.out.println("...wait done."); - } catch (InterruptedException ignore) {} + // System.out.println("...wait done."); + } catch (InterruptedException ignore) { + } /***/ } @@ -558,7 +595,7 @@ protected JComponent createMainPanel() { @Override protected void shutdown() { - //System.out.println("Shutdown."); + // System.out.println("Shutdown."); if (vscom != null) { try { vscom.put("trick.var_clear()"); @@ -611,7 +648,8 @@ protected Void doInBackground() { // Cyclic data as read from var server: // Time info: sim_time, mtv_update_ticker // Event info: - // (for each event) active, fired_time, fired_count, ran_time, ran_count, manual, manual_fired, hold, added + // (for each event) active, fired_time, fired_count, ran_time, ran_count, manual, manual_fired, hold, + // added // (for each condtion) enabled, fired_time, fired_count, hold // (for each action) enabled, ran_time, ran_count @@ -622,13 +660,15 @@ protected Void doInBackground() { continue; } if (need_update) { - // mtv forced an update (e.g. user deleted an event), need to udpate mtv event table before getting any more data + // mtv forced an update (e.g. user deleted an event), need to udpate mtv event table before + // getting any more data update(); continue; } results = vscom.get(mtv_var_count).split("\t"); if ((results == null) || (results.length != mtv_var_count)) { - System.out.println("Read unknown (length= " + results.length + " expected= " + mtv_var_count + "), exitting."); + System.out.println("Read unknown (length= " + results.length + " expected= " + mtv_var_count + + "), exitting."); break; } // if this is a one shot read during pause, don't process any further @@ -639,25 +679,25 @@ protected Void doInBackground() { } // Debug print out what you read /*** - System.out.print("Read results: length= " + results.length); - for (int ii=0; ii"); - } - System.out.println(); - ***/ - + * System.out.print("Read results: length= " + results.length); + * for (int ii=0; ii"); + * } + * System.out.println(); + ***/ index = 0; // index into results that we read // 1st 2 variables are sim time and mtv update time; they are long long but get them as double?? if (Double.parseDouble(results[index + 0]) / time_tics > sim_time) { sim_time = Double.parseDouble(results[index + 0]) / time_tics; mtv_view.simtime_text.setText(" " + ((Double) sim_time).toString()); - //System.out.println("simtime=" + sim_time); + // System.out.println("simtime=" + sim_time); } update_ticker = Integer.parseInt(results[index + 1]); - //System.out.println("update ticker=" +update_ticker); + // System.out.println("update ticker=" +update_ticker); if (update_ticker != last_update_ticker) { // event(s) have been added or deleted in user code, time to update mtv event table - //System.out.println("last update ticker=" +last_update_ticker+ " update_ticker=" +update_ticker); + // System.out.println("last update ticker=" +last_update_ticker+ " update_ticker=" + // +update_ticker); last_update_ticker = update_ticker; update(); continue; @@ -678,8 +718,8 @@ protected Void doInBackground() { time = Double.parseDouble(results[index + 1]); count = Integer.parseInt(results[index + 2]); fired = (mtv_view.event_table.getValueAt(rownum, 3) == null) - ? count > 0 - : count > (Integer) mtv_view.event_table.getValueAt(rownum, 3); + ? count > 0 + : count > (Integer) mtv_view.event_table.getValueAt(rownum, 3); // set event fired column color red for a second remove_color = (mtv_view.is_red[rownum]) && (sim_time >= time + color_duration); if (fired || remove_color) { @@ -690,8 +730,8 @@ protected Void doInBackground() { time = Double.parseDouble(results[index + 3]); count = Integer.parseInt(results[index + 4]); ran = (mtv_view.event_table.getValueAt(rownum, 5) == null) - ? count > 0 - : count > (Integer) mtv_view.event_table.getValueAt(rownum, 5); + ? count > 0 + : count > (Integer) mtv_view.event_table.getValueAt(rownum, 5); // set event ran column color green for a second remove_color = (mtv_view.is_green[rownum]) && (sim_time >= time + color_duration); if (ran || remove_color) { @@ -738,8 +778,10 @@ protected Void doInBackground() { for (int jj = 0; jj < mtv_events.get(map[ii]).cond_count; jj++) { // CONDITION if (event_active_state_changed) { - // the event's Active or Added was clicked, so simply touch row so that it is rendered again - mtv_view.event_table.setValueAt(mtv_view.event_table.getValueAt(rownum, 0), rownum, 0); + // the event's Active or Added was clicked, so simply touch row so that it is + // rendered again + mtv_view.event_table.setValueAt( + mtv_view.event_table.getValueAt(rownum, 0), rownum, 0); } if (mtv_view.mode[mtv_events.get(mtv_view.eventmap[rownum]).row] != Mode.NORMAL) { // disable condition Active button in manual mode @@ -754,8 +796,8 @@ protected Void doInBackground() { time = Double.parseDouble(results[index + 1]); count = Integer.parseInt(results[index + 2]); fired = (mtv_view.event_table.getValueAt(rownum, 3) == null) - ? count > 0 - : count > (Integer) mtv_view.event_table.getValueAt(rownum, 3); + ? count > 0 + : count > (Integer) mtv_view.event_table.getValueAt(rownum, 3); // set condition fired column color red for a second remove_color = (mtv_view.is_red[rownum]) && (sim_time >= time + color_duration); if (fired || remove_color) { @@ -773,8 +815,10 @@ protected Void doInBackground() { for (int jj = 0; jj < mtv_events.get(map[ii]).act_count; jj++) { // ACTION if (event_active_state_changed) { - // the event's Active or Added was clicked, so simply touch row so that it is rendered again - mtv_view.event_table.setValueAt(mtv_view.event_table.getValueAt(rownum, 0), rownum, 0); + // the event's Active or Added was clicked, so simply touch row so that it is + // rendered again + mtv_view.event_table.setValueAt( + mtv_view.event_table.getValueAt(rownum, 0), rownum, 0); } mtv_view.active[rownum] = Integer.parseInt(results[index + 0]) == 1; if (!mtv_view.event_table.getValueAt(rownum, 0).equals(mtv_view.active[rownum])) { @@ -783,8 +827,8 @@ protected Void doInBackground() { time = Double.parseDouble(results[index + 1]); count = Integer.parseInt(results[index + 2]); ran = (mtv_view.event_table.getValueAt(rownum, 5) == null) - ? count > 0 - : count > (Integer) mtv_view.event_table.getValueAt(rownum, 5); + ? count > 0 + : count > (Integer) mtv_view.event_table.getValueAt(rownum, 5); // set action ran column color green for a second remove_color = (mtv_view.is_green[rownum]) && (sim_time >= time + color_duration); if (ran || remove_color) { @@ -810,12 +854,12 @@ protected Void doInBackground() { @Override protected void succeeded(Void ignored) { - //System.out.println("Succeeded."); + // System.out.println("Succeeded."); } @Override protected void finished() { - //System.out.println("Finished."); + // System.out.println("Finished."); if (vscom != null) { try { vscom.put("trick.var_clear()"); diff --git a/trick_source/sim_services/MemoryManager/MemoryManager_ref_dim.cpp b/trick_source/sim_services/MemoryManager/MemoryManager_ref_dim.cpp index bbae17221..a8e44625f 100644 --- a/trick_source/sim_services/MemoryManager/MemoryManager_ref_dim.cpp +++ b/trick_source/sim_services/MemoryManager/MemoryManager_ref_dim.cpp @@ -267,12 +267,15 @@ int Trick::MemoryManager::ref_dim( REF2* R, V_DATA* V) { return (TRICK_PARAMETER_ARRAY_SIZE); } } else { // Dynamic array case - // Check if the index is out of bounds if the pointer points to a dynamic array - if (index_value >= (get_size(*(void**)(R->address)))) { + // Check if the index is out of bounds if the pointer points to a dynamic array. + // R->address is already dereferenced to the heap array. + if (index_value >= (get_size(R->address))) + { /* print out of bounds error message if MM debug_level is greater than 1 */ if (debug_level > 1) { std::stringstream message; - message << index_value << " is out of bounds for " << R->reference << " (size=" << get_size(*(void**)(R->address)) << ")."; + message << index_value << " is out of bounds for " << R->reference + << " (size=" << get_size(R->address) << ")."; emitError(message.str()); } free(ref2);