#154 fix MAPIERROR typemap

[Nick Czeczulin]

This patch has 2 fixes:
1. missing null ptr check that can cause a crash.
2. $target was never assigned with a result.

While this patch will make the various mapi GetLastError() calls work now, it requires MAPI_UNICODE to always be passed during their calls in order to yield a valid result. While likely to be satisfactory, GetLastError() can return with MAPI_E_BAD_CHARWIDTH. A future patch making MAPI_UNICODE optional and avoiding the use of the typemap would be ideal.

[Mark Hammond]

I don't understand the:

• if ($target == Py_None)
• Py_DECREF(Py_None);

part of this - it smells wrong as it shouldn't be possible for us to have an extra reference hanging around but only when the object is None.

I'm also not clear on the status of this and #156 - if we take that, can we just drop this?

[Nick Czeczulin]

Hi Mark,

Yes. We can drop #154. I believe #156 removes any existing dependency
for this typemap.

Regarding the Py_DECREF(Py_None), I thought it was needed because of the
following typemap:

%typemap(python,out) HRESULT {
$target = Py_None;
Py_INCREF(Py_None);
}

which ends up initializing the _resultobj in the generated .cpp?
{
_resultobj = Py_None;
Py_INCREF(Py_None);
}

On 2/26/2016 6:27 PM, Mark Hammond wrote:

I don't understand the:

• if ($target == Py_None)
• Py_DECREF(Py_None);

part of this - it smells wrong as it shouldn't be possible for us to have an extra reference hanging around but only when the object is None.

I'm also not clear on the status of this and #156 - if we take that, can we just drop this?

---

[patches:#154] fix MAPIERROR typemap

Status: open
Group: Unstable (example)
Created: Wed Oct 14, 2015 06:50 AM UTC by Nick Czeczulin
Last Updated: Wed Oct 14, 2015 06:50 AM UTC
Owner: nobody
Attachments:

• fix-MAPIERROR-typemap.diff (824 Bytes; application/octet-stream)

This patch has 2 fixes:
1. missing null ptr check that can cause a crash.
2. $target was never assigned with a result.

While this patch will make the various mapi GetLastError() calls work now, it requires MAPI_UNICODE to always be passed during their calls in order to yield a valid result. While likely to be satisfactory, GetLastError() can return with MAPI_E_BAD_CHARWIDTH. A future patch making MAPI_UNICODE optional and avoiding the use of the typemap would be ideal.

---

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/pywin32/patches/154/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

[Nick Czeczulin]

Yes. We can abandon this patch. I believe [#156] removes any existing dependency for this typemap.

Regarding the Py_DECREF(Py_None), I thought it was needed because of the following typemap:

%typemap(python,out) HRESULT {
    $target = Py_None;
    Py_INCREF(Py_None);
}

which ends up initializing the _resultobj in the generated .cpp with?

{
    _resultobj = Py_None;
    Py_INCREF(Py_None);
}

[Mark Hammond]

Hrm - #149 is doing the same basic thing. I can see in the generated code that we are leaking that reference to None, but I'm not sure this is the best way to fix it. A leak of None isn't that bad, so I'm going to defer this for a while in the interests of getting some other patches in.

[Mark Hammond]

Yeah, but it still seems something is wrong with the swig types there. HRESULT is typically used when the HRESULT is discarded, while HRESULT_KEEP will return the hresult. So at face value, it seems that |%typemap(python,out) HRESULT {...| code is wrong (ie, it should be an empty block) - but no doubt it's not going to be that simple. I'm afraid I'm out of time for today though.