New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: xd3_encode_input() misbehaves when input data is fed in small chunks #79

Closed
GoogleCodeExporter opened this Issue Mar 24, 2015 · 9 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Mar 24, 2015

Repro steps:
1. create a small app that feeds the input (new file) in blocks
2. tweak the block size so the input fits into one block - encode 
operation succeeds
3. decrease the block so it takes several invocations of xd3_encode_input
() to process the data - encode operation writes an invalid diff out

WindowsXP/VS2005, xdelta_3.0u as well as the current SVN version.

Basically the issue manifests itself in the piece of below. Everything 
works correctly as long as the "input" buffer contains the whole input 
file. Otherwise, when the buffer is small and xd3_avail_input() is called 
multiple times, the encode operation produces invalid output. 

    for (;;)
    {
        byte input[0x1000];
        if (!ReadFile(file, input, sizeof(input), &inputSize, 
NULL))
            return EXIT_FAILURE;

        readSoFar += inputSize;

        // If we've reached EOF tell the stream to flush
        if (readSoFar == filesize)
            stream.flags |= XD3_FLUSH;

        xd3_avail_input(&stream, input, inputSize);

process:
        ret = xd3_encode_input(&stream);

Original issue reported on code.google.com by s0laria...@gmail.com on 25 Oct 2008 at 6:51

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

Below is a nicer, cross-platofrm version of my main stream-processing loop. 
It's 
based on your app, I implemented the GetBlock() function to access the source 
file. 
In any case, that part seems to behave, and chaning the input block size breaks 
my 
app.

Am I missing something vital in my code or is it an issue in the lib?



    std::fstream istream(ifile->filename, std::ios_base::in | 
std::ios_base::binary);
    if (!istream.is_open())
        return EXIT_FAILURE;

    for (;;)
    {
        byte input[0x1000];

        // Read a buffer full of data
        istream.read((char *)input, sizeof(input));

        // If we've reached EOF tell the stream to flush
        if (istream.eof())
            stream.flags |= XD3_FLUSH;

        xd3_avail_input(&stream, input, istream.gcount());

process:
        ret = xd3_encode_input(&stream);

        if (ret == XD3_INPUT)
        {
            if (istream.eof())
                break;
            else
                continue;
        }

        if (ret == XD3_WINSTART)
            goto process;
        else if (ret == XD3_OUTPUT)
        {
            /* Defer opening the output file until the stream produces 
its
             * first output
             */
            if (ofile != NULL &&
                !main_file_isopen(ofile) &&
                (ret = main_open_output(&stream, ofile)) != 0)
            {
                return EXIT_FAILURE;
            }

            if (ret = main_write_output(&stream, ofile))
                return EXIT_FAILURE;

            ret = 0;

            xd3_consume_output(&stream);
            goto process;
        }
        else if (ret == XD3_WINFINISH)
            goto process;
        else 
            return EXIT_FAILURE;
    }

Original comment by s0laria...@gmail.com on 26 Oct 2008 at 1:16

GoogleCodeExporter commented Mar 24, 2015

Below is a nicer, cross-platofrm version of my main stream-processing loop. 
It's 
based on your app, I implemented the GetBlock() function to access the source 
file. 
In any case, that part seems to behave, and chaning the input block size breaks 
my 
app.

Am I missing something vital in my code or is it an issue in the lib?



    std::fstream istream(ifile->filename, std::ios_base::in | 
std::ios_base::binary);
    if (!istream.is_open())
        return EXIT_FAILURE;

    for (;;)
    {
        byte input[0x1000];

        // Read a buffer full of data
        istream.read((char *)input, sizeof(input));

        // If we've reached EOF tell the stream to flush
        if (istream.eof())
            stream.flags |= XD3_FLUSH;

        xd3_avail_input(&stream, input, istream.gcount());

process:
        ret = xd3_encode_input(&stream);

        if (ret == XD3_INPUT)
        {
            if (istream.eof())
                break;
            else
                continue;
        }

        if (ret == XD3_WINSTART)
            goto process;
        else if (ret == XD3_OUTPUT)
        {
            /* Defer opening the output file until the stream produces 
its
             * first output
             */
            if (ofile != NULL &&
                !main_file_isopen(ofile) &&
                (ret = main_open_output(&stream, ofile)) != 0)
            {
                return EXIT_FAILURE;
            }

            if (ret = main_write_output(&stream, ofile))
                return EXIT_FAILURE;

            ret = 0;

            xd3_consume_output(&stream);
            goto process;
        }
        else if (ret == XD3_WINFINISH)
            goto process;
        else 
            return EXIT_FAILURE;
    }

Original comment by s0laria...@gmail.com on 26 Oct 2008 at 1:16

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

Here is debug output from two "encode" runs - the working one (bigger buffer) 
and 
broken one (smaller buffer)

Original comment by s0laria...@gmail.com on 27 Oct 2008 at 11:58

Attachments:

GoogleCodeExporter commented Mar 24, 2015

Here is debug output from two "encode" runs - the working one (bigger buffer) 
and 
broken one (smaller buffer)

Original comment by s0laria...@gmail.com on 27 Oct 2008 at 11:58

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

I need to see your entire program.  How are you setting xd3_config options?  I 
think
winsize may be incorrect.

Original comment by josh.mac...@gmail.com on 28 Oct 2008 at 1:38

GoogleCodeExporter commented Mar 24, 2015

I need to see your entire program.  How are you setting xd3_config options?  I 
think
winsize may be incorrect.

Original comment by josh.mac...@gmail.com on 28 Oct 2008 at 1:38

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

Here is the encode piece. Thanks!

Original comment by s0laria...@gmail.com on 28 Oct 2008 at 4:24

Attachments:

GoogleCodeExporter commented Mar 24, 2015

Here is the encode piece. Thanks!

Original comment by s0laria...@gmail.com on 28 Oct 2008 at 4:24

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

...also, the window size was printed in both runs there: 
http://code.google.com/p/xdelta/issues/detail?id=79#c2

Original comment by s0laria...@gmail.com on 28 Oct 2008 at 4:26

GoogleCodeExporter commented Mar 24, 2015

...also, the window size was printed in both runs there: 
http://code.google.com/p/xdelta/issues/detail?id=79#c2

Original comment by s0laria...@gmail.com on 28 Oct 2008 at 4:26

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

FWIW, I've modified testing/regtest.cc to expose this issue.  I'll fix it soon.

Original comment by josh.mac...@gmail.com on 10 Feb 2009 at 3:50

  • Changed state: Accepted
  • Added labels: Priority-High
  • Removed labels: Priority-Medium

GoogleCodeExporter commented Mar 24, 2015

FWIW, I've modified testing/regtest.cc to expose this issue.  I'll fix it soon.

Original comment by josh.mac...@gmail.com on 10 Feb 2009 at 3:50

  • Changed state: Accepted
  • Added labels: Priority-High
  • Removed labels: Priority-Medium
@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

The immediate problem is fixed in SVN 280.  However, I think there is one 
lingering
issue and I don't want to cut a release until I add another test.

BTW, the root cause of this issue being triggered is that your window size is 
larger
than your input buffer size, so xdelta is buffering your input for you.  It's
provided as a convenience, but to be the most efficient you may consider using a
buffer size equal to your window size.  In your example, you had buffer_size = 
0x1000
and winsize = 0x4000.  Try making those equal and you will bypass this
buffering/leftover issue entirely.

Original comment by josh.mac...@gmail.com on 12 Feb 2009 at 3:42

GoogleCodeExporter commented Mar 24, 2015

The immediate problem is fixed in SVN 280.  However, I think there is one 
lingering
issue and I don't want to cut a release until I add another test.

BTW, the root cause of this issue being triggered is that your window size is 
larger
than your input buffer size, so xdelta is buffering your input for you.  It's
provided as a convenience, but to be the most efficient you may consider using a
buffer size equal to your window size.  In your example, you had buffer_size = 
0x1000
and winsize = 0x4000.  Try making those equal and you will bypass this
buffering/leftover issue entirely.

Original comment by josh.mac...@gmail.com on 12 Feb 2009 at 3:42

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

Thank you. I'll try running with matching buffer and window sizes.

Original comment by s0laria...@gmail.com on 26 Feb 2009 at 11:07

GoogleCodeExporter commented Mar 24, 2015

Thank you. I'll try running with matching buffer and window sizes.

Original comment by s0laria...@gmail.com on 26 Feb 2009 at 11:07

@GoogleCodeExporter

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 24, 2015

SVN 281 has the definitive fix for this issue, and a test.

Original comment by josh.mac...@gmail.com on 8 Mar 2009 at 3:40

  • Changed state: Fixed

GoogleCodeExporter commented Mar 24, 2015

SVN 281 has the definitive fix for this issue, and a test.

Original comment by josh.mac...@gmail.com on 8 Mar 2009 at 3:40

  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment