asc file read exception with empty lines
Thank you for this wonderful library and the work you’re putting in, as I was working with this asc reader I came across a peculiuar situation, I was reading the asc file (strangely, a file from the unittest folder, so a standard file) and an exception was thrown when I was parsing the different asc lines.
Exception location,
in File.cpp read() method,
when the reader encounters an empty line (last line of asc file), an exception is thrown when trying the check line.back()=='\r'
I managed to work around this issue by adding a return nullptr when line is empty before this check, but an official fix would be wonderful
Thank you for your support.
Comments (18)
-
repo owner -
repo owner Hello Houssem,
in which test file you see the exception?
Can this be a matter of line endings, as the line.back() test just checks Windows line endings and not Linux.
Bye
Tobias
-
reporter I saw this issue in all the test files I tried, I think it’s caused by the last line of the asc file being empty, the end of file flag was still false, but there was no more data to be parsed by the read() method
-
repo owner Hi Houssem,
I still cannot reproduce it here. There are no exceptions or hangs in the existing test cases due to empty lines here.
Let’s exchange on the environment. I’m developing and testing using:
- OS: Linux
- Compiler: gcc 10.2 or clang 9.0
- Flex: v2.6.4
What’s your environment? I’m going to setup this environment here as well to reproduce the issue.
Bye
Tobias
-
repo owner -
assigned issue to
-
assigned issue to
-
repo owner - changed status to open
-
repo owner The lexer is configured to return EventType::Unknown on all unknown lines including empty ones.
If the exception occurs in the line, where “line.back() == ‘\r’”, this probably means that in the line above “std::string line = scanner->YYText()” a nullptr was assigned to line. I can make a change here to prevent this. Maybe this solves the issue already.
-
reporter I don’t think it’s environment specific
My environment- OS: windows 10
- Compiler : Microsoft (R) C/C++ Optimizing Compiler Version 19.27.29111 (visual studio 19 compiler)
- Flex : v2.6.4
maybe it’s an issue with end of file not being set,
what I did exactly was loop untill file.eof() is hit and in the loop call file.read()
while(!file.eof()){ packet = file.read() }
when I debugged the “std::string line = scanner->YYText()”, line value was empty string ““.
-
repo owner Hi Houssem,
I pushed a couple of changes into branch “issue-6”. The last one contains the proposed change. Can you check if this solves the issue?
https://bitbucket.org/tobylorenz/vector_asc/branch/issue-6
Bye
Tobias
-
reporter I checked out the branch and tried my scenario with and the problem still persists (exception still appears)
-
repo owner Ok, the documentation of std::string back() says “This function shall not be called on empty strings.”. “Otherwise, it causes undefined behavior.”.
So, I would some code to prevent this. Let’s see if this solves the issue.
-
repo owner A commit the proposed change to the issue-6 branch. The behavior on empty lines is now that File::read() returns nullptr. Might be that it returned Event::Unknown before.
Can you check if this solves the issue?
-
reporter the exception is still reproducible, the value of scanner->YYleng() is 1 and not 0, maybe it would be easier to just reason on the line value (if it’s an empty line return nullptr)
-
repo owner Ah, yyleng is likely 1, because line='\r'.
I pushed another change that handles
- YYText() being nullptr, and
- line empty
Returning nullptr is not appropriate in this case, because that’s the signal for end of line. Instead Event::Unknown with the empty line should be returned.
I guess, I have to squash this change series now.
So does this work for you?
Bye
Tobias
-
reporter Yess, I ran my code with the lates code update and it worked without an exception
Many thanks.
-
repo owner Great. I’ll squash the changes and merge them into master. Will be released as a fix release soon.
Thanks for testing,
Tobias
-
reporter It was a pleasure, again, thank you for the great work you’re doing
-
repo owner - changed status to resolved
Solved in release v1.1.3
- Log in to comment
Hello Houssem,
I’ll take a look and correct it. Shouldn’t be too difficult to make this fix.
However it might still take some time, due upcoming vacation period
Bye
Tobias