Skip to content
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

Error with mpack_writer_destroy when no data has been written #58

Closed
volks73 opened this issue Oct 18, 2017 · 3 comments
Closed

Error with mpack_writer_destroy when no data has been written #58

volks73 opened this issue Oct 18, 2017 · 3 comments

Comments

@volks73
Copy link
Contributor

volks73 commented Oct 18, 2017

I have been working with the mpack library, and I have come across an error. It appears to be related to using the mpack_writer_destroy function when no other mpack_write_* functions have been used. In other words, an error occurs destroying a writer with no data. I understand this is an edge case because why would one create and then ultimately destroy a writer without using it? However, it feels like mpack should exit cleanly/safely as opposed to causing an abort under these conditions.

I have created a simplified example that reproduces the problem on my machine. I was not sure how to convert it to a test to include with the mpack test suite. Instead, I have included the example as an attachment, along with a screen capture of the error dialog that appears (Error-Dialog.png). Below is the source code for the main function that reproduces the problem, which is located in issue.c of the attached archive:

#include "mpack.h"

int 
main(int argc, char* argv[])
{
    char* data = NULL;
    size_t size = 0;
    mpack_writer_t* writer = malloc(sizeof(mpack_writer_t));
    if (!writer) {
        fprintf(stderr, "A memory-related error occurred!\n");
        return 1;
    }
    mpack_writer_init_growable(writer, &data, &size);
    if (mpack_writer_error(writer) != mpack_ok) {
        fprintf(stderr, "An initialization-related error occurred!\n");
        return 1;
    }
    //mpack_write_int(writer, 56); <-- Uncomment this line, i.e. write some data, to avoid the issue
    if (mpack_writer_destroy(writer) != mpack_ok) {
        fprintf(stderr, "A destruction-related error occurred!\n");
        return 1;
    }
    fprintf(stdout, "Data:");
    for (size_t i = 0; i < size; i++) {
        fprintf(stdout, " %X", data[i]);
    }
    fprintf(stdout, "\n");
    free(data);
    free(writer);
    return 0;
}

The error does not appear to occur on macOS High Sierra. It only occurs on Windows 10, where I am using the MSVC compiler included with Microsoft C++ Build Tools 2017. I have not tried it on Linux. I have created a CMakeLists.txt file to help with building the example and included it in the attached archive file. I am using mpack v0.8.2 with the default configuration (mpack-config.h) in the example (also included in the attached archive). The following instructions can be used to build the example demonstrating the issue:

  1. Download the attached archive.
  2. Extract the archive.
  3. Start the x64 Native Build Tools command prompt that should have been included with Microsoft C++ Build Tools 2017.
  4. Navigate to the extracted archive folder.
  5. Run the following commands to build and run the example:
mkdir build
cd build
cmake ..
cmake --build .
Debug\run_issue.exe

I believe I have narrowed down the problem to the mpack_growable_writer_teardown function and its use of the mpack_realloc function, but I am not sure on how to resolve the issue, or if this is even an error. Maybe it is just a configuration I need to change? It is very possible I have missed something.

issue.zip

error-dialog

@ludocode
Copy link
Owner

Thanks for the excellent bug report.

This bug has been fixed on develop back in April in 083e28d. Unfortunately it's been far too long since I've done a release, so it's still unfixed in the latest release version. I'm still working on a new release, but I've been delaying it since I keep trying to add more features (most recently timestamp support, which you can see in the timestamp branch here.)

In the meantime you could upgrade to the develop branch, or cherry-pick that commit over to the v0.8.1 tag. The tools/amalgamate.sh script can be used to generate new mpack.h and mpack.c files.

@volks73
Copy link
Contributor Author

volks73 commented Oct 23, 2017

Great! I am glad it was already resolved, and I am sorry for the redundant issue.

In the meantime you could upgrade to the develop branch, or cherry-pick that commit over to the v0.8.1 tag. The tools/amalgamate.sh script can be used to generate new mpack.h and mpack.c files.

Thanks for the temporary workaround. I look forward to the next revision/release.

@ludocode
Copy link
Owner

Oops, forgot to close this back when I released MPack 1.0. This is fixed in the latest release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants