dragokas commented 3 years ago

Help us help you

[x] I have checked that my issue doesn't exist yet.
[x] I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
[x] I can always reproduce the issue with the provided description below.

Environment

Current SourceMod version: 1.11.0.6675
Current SourceMod snapshot: 1.11.0.6675
Current Metamod: Source snapshot: 1.10.7

Description

Similarly to this issue: https://github.com/alliedmodders/sourcemod/issues/1430

Expected behavior

Imagine, we can't strongly control the input file contents, and we're not absolutely sure what could be the max line length. Each time we're executing File.ReadLine, we're expecting that next line will be read.

Actual behavior

However, if buffer too small to hold the entire line, the next time File.ReadLine will return the next piece of the same 1st line, not the second line. Also, we don't have any indicator saying have we read the entire line to have an ability for skipping the leftover.

Problematic Code (or Steps to Reproduce)

public void OnPluginStart()
{
    char str[4]; // too small buffer
    File hr = OpenFile("addons/test.txt", "rt");
    if( hr )
    {
        while( !hr.EndOfFile() && hr.ReadLine(str, sizeof(str)) )
        {
            // next time it will print the piece of the same line (if the buff. size < line length in the file)
            PrintToServer("Line: %s", str);
        }
        delete hr;
    }
}

asherkin commented 3 years ago

This is the expected behaviour for ReadLine (unlike the binary-safe ReadString) - it matches common fgets behaviour.

Headline commented 3 years ago

Perhaps documentation could be changed to better express this behavior, but as asher mentioned this is indeed intended. What do you think @dragokas?

dragokas commented 3 years ago

@Headline Since both, this issue and https://github.com/alliedmodders/sourcemod/issues/1502 are expected behaviour for all these years, I think devs already used to it and consider in their plugins, but for novice it must be explicitly indicated in the documentation.

Both "issues" are complement themselves, so I think such 3 statements should be appended to the documentation (I'm sure you can explain it in docs much more clearly):

1) File.ReadLine includes line ending (CR LF) characters in destination buffer, if it is large enough to hold the entire line. If you want to trim such characters from the buffer, it is useful to apply TrimString().

2) If destination buffer cannot hold the entire line, the file pointer is not populated to the end of line automatically, instead the next File.ReadLine operation will start reading the same line from where it ends last time.

3) When you want to check if the line get read in buffer entirely, you can verify the last character in that buffer for LF (0xA). It doesn't necassarily happen when the file pointer reached EOF.

Headline commented 3 years ago

I agree - lets transform this issue into a documentation change request

KyleSanderson commented 3 years ago

There's another issue here, feof (EndOfFile) is undefined until the buffer has been read. This can cause early bailouts and other oddness.

alliedmodders / sourcemod