autowarefoundation / autoware.universe

https://autowarefoundation.github.io/autoware.universe/
Apache License 2.0
937 stars 614 forks source link

Suspicious for-loop in `diagnostic_graph_aggregator` #6928

Closed veqcc closed 4 months ago

veqcc commented 4 months ago

Checklist

Description

@isamu-takagi Hi, I have a question to diagnostic_graph_aggregator maintainer.

The cppcheck, a static analyzer, reports like the following:

system/diagnostic_graph_aggregator/src/common/graph/config.cpp:235:18: error: Calling '=' while iterating the container is invalid. [invalidContainerLoop]
      unit->list = filtered_list; 
                 ^         
system/diagnostic_graph_aggregator/src/common/graph/config.cpp:231:5: note: Iterating container here.
    for (const auto & link : unit->list) {
    ^                                                                           
system/diagnostic_graph_aggregator/src/common/graph/config.cpp:235:18: note: Calling '=' while iterating the container is invalid. 
      unit->list = filtered_list;
                 ^

The actual code looks like this:

    std::vector<LinkConfig *> filtered_list;
    for (const auto & link : unit->list) {
      if (remove_links.count(link) == 0) {
        filtered_list.push_back(link);
      }
      unit->list = filtered_list;
    }

https://github.com/autowarefoundation/autoware.universe/blob/aab29f7c35132527c8efe72e254bde96f251d6bc/system/diagnostic_graph_aggregator/src/common/graph/config.cpp#L231C5-L236C6

As the cppcheck detects, unit->list should not be updated while being iterated. However, I couldn't figure out what the expected behavior here and couldn't make a fixing PR. Could you explain the expected behavior or make a modification instead?

Expected behavior

See the description.

Actual behavior

See the description.

Steps to reproduce

See the description.

Versions

No response

Possible causes

No response

Additional context

No response

isamu-takagi commented 4 months ago

@veqcc Thank you. I fixed it. https://github.com/autowarefoundation/autoware.universe/pull/6932