Xml_generator: prevent unnecessary new lines #2967
Comments
|
This issue title, description, and label leaves me puzzled. It suggests that something fundamental is broken and must be fixed but remains completely unspecific about the symptom and nature of the problem. If the issue is about the topic we discussed offline just yesterday, an appropriate title would be "Xml_generator: improve formatting on exception" with the label "cleanup" attached. From what I know, the issue is merely about a formatting improvement in the exception case, which happened to become a relevant in your case where exceptions are used in a non-exceptional way. I want to encourage you to spend a little more effort on meaningful issue titles, descriptions, and labels. An issue should at least be specific about the symptom and the expected behavior. Please keep in mind that the issue tracker is part of our project's documentation. |
1) The loop for determining the line length read from a character offset before checking whether the offset is smaller than the given string length. This could have caused access outside the string buffer. 2) The routine for determining the line length first seeked for the offset of the last real character of the line and than added one for getting the length but only if the following character was '\n'. This has to be done for any other line-terminating character too. The only case where you don't want to do this is when the end of the whole string is reached. Issue genodelabs#2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue genodelabs#2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes genodelabs#2967
|
@nfeske Sorry for that. I thought it would be fixed right away but it turned out to be more time-intensive. |
m-stein
changed the title
|
The commits are very insightful. Thanks for the good commit messages and the extended tests. |
1) The loop for determining the line length read from a character offset before checking whether the offset is smaller than the given string length. This could have caused access outside the string buffer. 2) The routine for determining the line length first seeked for the offset of the last real character of the line and than added one for getting the length but only if the following character was '\n'. This has to be done for any other line-terminating character too. The only case where you don't want to do this is when the end of the whole string is reached. Issue #2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue #2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes #2967
|
Related to the discussion of genode/repos/os/src/test/xml_generator/main.cc Lines 66 to 69 in 9dc4c9d |
|
I suspect the commit series breaks |
|
The commit |
chelmuth
changed the title
|
Let's keep this line of work for merging at a later point because it deserves more attention than I'm able to provide right now. @m-stein I'm wondering, in the commit message you state "Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML." as a fact. But have you actually witnessed this sequence in the wild? I'm asking because I haven't (looking at the reports in sculpt's report fs), and when looking at the code, I would not expect it either. Let my try to clarify, the BTW, The ...to be investigated later. |
1) The loop for determining the line length read from a character offset before checking whether the offset is smaller than the given string length. This could have caused access outside the string buffer. 2) The routine for determining the line length first seeked for the offset of the last real character of the line and than added one for getting the length but only if the following character was '\n'. This has to be done for any other line-terminating character too. The only case where you don't want to do this is when the end of the whole string is reached. Issue #2967
1) The loop for determining the line length read from a character offset before checking whether the offset is smaller than the given string length. This could have caused access outside the string buffer. 2) The routine for determining the line length first seeked for the offset of the last real character of the line and than added one for getting the length but only if the following character was '\n'. This has to be done for any other line-terminating character too. The only case where you don't want to do this is when the end of the whole string is reached. Issue #2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue genodelabs#2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes genodelabs#2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue genodelabs#2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes genodelabs#2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue genodelabs#2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes genodelabs#2967
When the functor provided to the Node constructor throws an exception, do revert all changes in reverse order. Previously, the changes made to the parent node were not considered by the exception handler which caused unnecessary characters to remain in the out buffer for each reverted node. Issue genodelabs#2967
Previously, a '\0' was appended to the out buffer at the end of each call of the Node constructor. At the end of the Xml_generator constructor, at the other hand, '\n' was appended. Without exceptions in the Node constructor this led to "\0\n" at the end of generated XML. However, with exceptions, the '\0' of the last Node constructor call was sometimes overwritten, leading to a mere "\n" at the end. It is better to append '\0' only in one place, namely at the point where the '\n' was appended in the Xml_generator constructor. This also fixes the problem with the exceptions. This also extends the xml_generator test to drive a harder test on exceptions in the Xml_generator. Fixes genodelabs#2967
|
Fixed in master. |
The handling of exceptions in the Xml_generator::Node constructor leaves unwanted new lines in the XML string.
The text was updated successfully, but these errors were encountered: