grass
grass copied to clipboard
OSGeo
lib: Support all newlines in G_getl
Use G_getl2 in G_getl to have everywhere the same behavior of supporting all newlines. Originally, G_getl2 was created to keep the behavior of G_getl. However, now, we want G_getl2 behavior everywhere.
Keeping G_getl2 for compatibility. Code can later be clean up to use only G_getl, probably at the time when G_getl2 is removed (v9).
The new test fails without the change in the library for CR and CRLF for G_getl, but passes with the change (at least on Linux).
I created a LXC Ubuntu container and also observe test failure within the container. At the moment I have no idea of its cause, but there is a segfault when run under GDB and test runs fine under valgrind. (Use of uninitialized / unallocated memory?) @nilason maybe you see the error instantly?
@marisn or @nilason, if you agree that the issue is the test, I can split this into two PRs to get the new code in now and add the test hopefully later.
@marisn or @nilason, if you agree that the issue is the test, I can split this into two PRs to get the new code in now and add the test hopefully later.
I'll take a look on the test. Probably easier to track down locally.
I would perhaps just ignore it since it is CR, although it seems that according to a SE comment, still in 2012, you could get CR:
The problem is with Microsoft Excel. When you save a CSV file on a Mac, it uses CR.
I would perhaps just ignore it since it is CR, although it seems that according to a SE comment, still in 2012, you could get CR:
The problem is with Microsoft Excel. When you save a CSV file on a Mac, it uses CR.
It looks like the function never really worked as intended, just never was up to the test. It parses \r\n just fine.
I did not see the issue with G_getl2 in #3850 and I don't see with a quick look now. The idea was to merge this PR before #3850 to have the test in place, but I got stuck with the library issues in this PR. (If I recall correctly, we knew #3850 helps on Windows with CRLF, but I don't see that explicitly mentioned in the PR.)
I did not see the issue with G_getl2 in #3850 and I don't see with a quick look now. The idea was to merge this PR before #3850 to have the test in place, but I got stuck with the library issues in this PR. (If I recall correctly, we knew #3850 helps on Windows with CRLF, but I don't see that explicitly mentioned in the PR.)
It seems that fgets() doesn't stop with CR. Only with LF (\n).
I agree that in must be fgets. This is out of scope of this PR anyway, so I'm setting the tests to expect failure. Tests should be done soon. Please, review.