| nienhueser | |
| rahn |
| Marble |
In method MarbleModelPrivate::assignFillColors(const QString &filePath) :
If the inner loop, which is iterating over the GeoSceneGeodata set in a layer, finds valid GeoSceneGeodata in the layer, the variable data becomes nonnull. If the data is nonnull, it causes the outer loop to exit even when the source file of GeoSceneData and path of parsed file don't match ( the condition data->sourceFile() == filePath in inner loop ). This results in applying wrong GeoSceneGeodata to a GeoDataDocument.
| Lint Skipped |
| Unit Tests Skipped |
Good catch!
What happens if found is always false? Perhaps data (i.e. the variable declared outside of the loops) should only be assigned if sourceFile() == filePath. In other words, it looks like the found variable could be replaced by a temporary variable being directly assigned by the dynamic_cast and that variable is assigned to data if the if condition matches. Right?
Now data is called uninitialized in data->sourceFile(), no? I think shentey meant this instead:
diff --git a/src/lib/marble/MarbleModel.cpp b/src/lib/marble/MarbleModel.cpp index 967479496..e8d190cf4 100644 --- a/src/lib/marble/MarbleModel.cpp +++ b/src/lib/marble/MarbleModel.cpp @@ -759,8 +759,9 @@ void MarbleModelPrivate::assignFillColors(const QString &filePath) } for (auto dataset: layer->datasets()) { - data = dynamic_cast<const GeoSceneGeodata *>(dataset); - if (data != nullptr && data->sourceFile() == filePath) { + auto sceneData = dynamic_cast<const GeoSceneGeodata *>(dataset); + if (sceneData != nullptr && sceneData->sourceFile() == filePath) { + data = sceneData; break; } }
| src/lib/marble/MarbleModel.cpp | ||
|---|---|---|
| 762 ↗ | (On Diff #13552) | This does not work. data is used before it is assigned a non-nullptr. I indeed meant the solution provided by Earthwings. |