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

Any[] and Any[][] arguments are invalidated #29

Closed
9il opened this issue Aug 27, 2017 · 2 comments
Closed

Any[] and Any[][] arguments are invalidated #29

9il opened this issue Aug 27, 2017 · 2 comments

Comments

@9il
Copy link
Contributor

9il commented Aug 27, 2017

Any[] / Any[][] are copies of XLOPER12 structures, which was freed using xlFree (is important in case of strings such as NAN).

@9il
Copy link
Contributor Author

9il commented Aug 27, 2017

The following two functions cause the bug:

T fromXlOper(T, A)(LPXLOPER12 oper, ref A allocator) if(is(T == Any)) {
    // FIXME: deep copy
    return Any(*oper);
}
// apply a function to an oper of type xltypeMulti
// the function must take a boolean value indicating if the cell value
// is to be converted or not, and a reference to the cell value itself
package void apply(T, alias F)(ref XLOPER12 oper) {
    import xlld.xlcall: XlType;
    import xlld.xl: coerce, free;
    import xlld.wrap: dlangToXlOperType, isMulti, numOperStringBytes;
    import xlld.any: Any;
    version(unittest) import xlld.test_util: gNumXlCoerce, gNumXlFree;

    if(!isMulti(oper))
        throw applyTypeException;

    const rows = oper.val.array.rows;
    const cols = oper.val.array.columns;
    auto values = oper.val.array.lparray[0 .. (rows * cols)];

    foreach(const row; 0 .. rows) {
        foreach(const col; 0 .. cols) {

            auto cellVal = coerce(&values[row * cols + col]);

            // Issue 22's unittest ends up coercing more than test_util can handle
            // so we undo the side-effect here
            version(unittest) --gNumXlCoerce; // ignore this for testing

            scope(exit) {
////////////////////////////////////////////////////////////////////
                free(&cellVal); //////////////  <---  THIS!!!!
////////////////////////////////////////////////////////////////////
                // see comment above about gNumXlCoerce
                version(unittest) --gNumXlFree;
            }

            // try to convert doubles to string if trying to convert everything to an
            // array of strings
            const shouldConvert =
                (cellVal.xltype == dlangToXlOperType!T.Type) ||
                (cellVal.xltype == XlType.xltypeNum && dlangToXlOperType!T.Type == XlType.xltypeStr) ||
                is(T == Any);

            F(shouldConvert, row, col, cellVal);
        }
    }
}

@atilaneves
Copy link
Collaborator

Fixed by this PR.

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