Eliminate line-oriented XML parsing

[davidpanderson]

There are a few functions like copy_element_contents() that use fgets() instead of fgetc(). This means they work correctly only the the close tag is on a line by itself.
Change these functions to use fgetc().

[TheAspens]

Would this be the time to replace the custom XML parsing with a implementation like TinyXML-2 or Xerces?

[davidpanderson]

I don't think so. That would be a giant change,
and (after this issue is fixed) would have no benefit.

[TheAspens]

I can see the argument about it being a significant change and this not being the time to do it. And so I retract my objection.

However, for the future, I disagree about there being no benefit. XML parsing has excellent libraries that are fast, extremely well tested and handle all sorts of cases that BOINC's doesn't. We continue to find bugs and gaps in the XML parsing and have to make changes to them. Why not eliminate that work and replace with something already solved? This issue wouldn't exist if we had already moved to a standard library.

[AenBleidd]

Hello Kevin, I have plans in future to work on this and move xml parsing to some standard library Thanks Best regards, Vitalii Koshura 2017-08-21 16:56 GMT+03:00 Kevin Reed <notifications@github.com>:

…

I can see the argument about it being a significant change and this not being the time to do it. And so I retract my objection. However, for the future, I disagree about there being no benefit. XML parsing has excellent libraries that are fast, extremely well tested and handle all sorts of edge cases that BOINC's doesn't. We continue to find bugs and gaps in the XML parsing and have to make changes to them. Why not eliminate that work and replace with something already solved? This issue wouldn't exist if we had already moved to a standard library. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2041 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFZoenhIrzDkXEcojr0EIt4QDvEm142ks5saYyjgaJpZM4O4Fua> .

[davidpanderson]

I'd want to see a comparison in terms of a) performance and b) code complexity. It's not clear that the cost/benefit merits doing this.

[ChristianBeer]

This is not merely a problem of performance or code complexity. By using a standard library we would make it easier for new developers to contribute since they don't need to understand a custom homegrown XML parser that doesn't support attributes and comments and has some other peculiarities you wouldn't expect.

Switching to a standard library will also move complexity away from BOINC, making the BOINC code easier to understand. Using libxml2 for example is easy as they provide high level functions to parse and manipulate XML fragments. I used this library to implement a check for global preferences in our scheduler and it only took me a day including getting to know libxml2, writing standalone tests of my use-case and thoroughly testing the implementation. It is still working and rejects invalid global preferences regularly.

Another possibility with libxml2 for example is that it supports xpath, parsing into a tree and validating XML against a predefined schema. I guess other libraries offer similar benefits.

[TheAspens]

I think for the sake of the original request in this thread, making the change David suggests is reasonable given the relative size of the efforts.

I have opened issue #2070 for further discussion of the idea of replacing the entire implementation.

[davidpanderson]

There are currently 3,542 lines of code that interface to the XML parser. These would all have to be replaced. Opportunity cost.
The current parser handles attributes (like <global_preferences venue="home">).

[AenBleidd]

@davidpanderson, I do not think that it is bad to rewrite existing functionality to make in easier and more stable. For example, I want to spend some time working on this task. Because I want and I like such kind of jobs. I want to cover all of this with unit tests too. So i think it will very good for a project to use some existing libraries that can make our life better and easier. I do not force anyone to do this. It is just my choice, for example. So why this is bad? This will not break any existing functionality. This is just replacing custom and potentially buggy code with better one that is already tested in many projects. And this is good, I think. This is what I think about such tasks. Thanks Best regards, Vitalii Koshura 2017-08-26 3:18 GMT+03:00 David Anderson <notifications@github.com>:

…

There are currently 3,542 lines of code that interface to the XML parser. These would all have to be replaced. Opportunity cost. The current parser handles attributes (like <global_preferences venue="home">). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2041 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFZoUmJ0P0yMpfjEk6onL89YnPvs9EOks5sb2RigaJpZM4O4Fua> .