Page MenuHomeWildfire Games

Get XML syntax errors not only the first time that file is loaded
AcceptedPublic

Authored by elexis on Jun 12 2018, 10:54 AM.

Details

Reviewers
bb
Trac Tickets
#5222
Summary

If an XML file happens to have syntax error (such as the one in credentials.xml in rP21847), then CXeromyces::ConvertFile will parse that file for the first time, throw a syntax error and then save a cached version of that file that doesn't have a syntax error to the cache.
Further loads will then go for the valid cache file, hiding the syntax error and possibly misleading authors into assuming their XML files are valid (such as in that commit).

Test Plan

Correctness: Notice that not writing a syntactically valid (but possibly semnatically wrong) cache file meets the only requirement of the claimed task.
Completeness: Judge whether this can break under some edge cases, such as packaged mods.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6244
Build 10368: Vulcan BuildJenkins
Build 10367: arc lint + arc unit

Event Timeline

elexis created this revision.Jun 12 2018, 10:54 AM
elexis updated this revision to Diff 6753.Jun 12 2018, 11:04 AM

Add comment.

Vulcan added a subscriber: Vulcan.Jun 12 2018, 11:04 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/644/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/645/display/redirect

elexis updated the Trac tickets for this revision.Jun 12 2018, 11:09 AM
bb accepted this revision.Jun 12 2018, 4:13 PM
bb added a subscriber: bb.

Agreeing

If it would crash for mods, the mods would receive an error in the current code, thus not caching the file wouldn't change much there.

This revision is now accepted and ready to land.Jun 12 2018, 4:13 PM
Stan added a subscriber: Stan.Jun 12 2018, 4:21 PM

What happens when archiving ?

vladislavbelov added inline comments.
source/ps/XML/Xeromyces.cpp
176

I think it's better to call it validSource, because of the function result is xmb.