Menu

#154 fix MAPIERROR typemap

Unstable (example)
closed-out-of-date
nobody
None
5
2016-02-27
2015-10-14
No

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.

1 Attachments

Related

Old Patches - new patches via https://github.com/mhammond/pywin32/pulls: #154
Old Patches - new patches via https://github.com/mhammond/pywin32/pulls: #156

Discussion

  • Mark Hammond

    Mark Hammond - 2016-02-27

    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

      Nick Czeczulin - 2016-02-27

      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:

      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/

       

      Related

      Old Patches - new patches via https://github.com/mhammond/pywin32/pulls: #154

  • Nick Czeczulin

    Nick Czeczulin - 2016-02-27

    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);
    }
    
     

    Related

    Old Patches - new patches via https://github.com/mhammond/pywin32/pulls: #156

  • Mark Hammond

    Mark Hammond - 2016-02-27

    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

    Mark Hammond - 2016-02-27
    • status: open --> closed-out-of-date
     
  • Mark Hammond

    Mark Hammond - 2016-02-27

    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.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.