Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Remove conversions.sip conditionals meant for 4.12 but actually 4.18
- The hex version of 4.12 was not 0x041200 (current 4.18) but 0x040c00.
  Code was always skipped and has worked with recent sip versions.
  • Loading branch information
dakcarto committed Apr 16, 2016
1 parent c921dc3 commit fc6559a
Showing 1 changed file with 0 additions and 29 deletions.
29 changes: 0 additions & 29 deletions python/core/conversions.sip
Expand Up @@ -680,11 +680,6 @@ template<TYPE>
PyObject *kobj, *tobj, *kobj2, *tobj2;
Py_ssize_t i = 0;

//TODO: it works using SIP
#if (SIP_VERSION >= 0x041200)
const sipMappedType* qmap2 = sipFindMappedType("QMap<int, TYPE>");
#endif

// Check the type if that is all that is required.
if (sipIsErr == NULL)
{
Expand All @@ -696,17 +691,12 @@ template<TYPE>
if (!PyDict_Check(tobj))
return 0;

#if (SIP_VERSION >= 0x041200)
if (!sipCanConvertToMappedType(tobj, qmap2, SIP_NOT_NONE))
return 0;
#else
Py_ssize_t j = 0;
while (PyDict_Next(tobj, &j, &kobj2, &tobj2))
{
if (!sipCanConvertToType(tobj2, sipType_TYPE, SIP_NOT_NONE))
return 0;
}
#endif
}
return 1;
}
Expand All @@ -717,24 +707,6 @@ template<TYPE>
{
qint64 k = PyLong_AsLongLong(kobj);

#if (SIP_VERSION >= 0x041200)
// TODO: search for the minimum SIP version this code works on, it works
// on SIP 4.13.3 (GS). See #else to know why the version check is needed.

int state;

TYPE* t = reinterpret_cast<TYPE*>(sipConvertToMappedType(tobj, qmap2, sipTransferObj,SIP_NOT_NONE,&state,sipIsErr));

if (*sipIsErr)
{
sipReleaseMappedType(t, qmap2, state);
delete qm;
return 0;
}

qm.insert(k, *t);
sipReleaseMappedType(t, qmap2, state);
#else
// using sipConvertToMappedType to convert directly to QMap<int, TYPE> doesn't work
// and ends with a segfault

Expand All @@ -759,7 +731,6 @@ template<TYPE>
sipReleaseType(t2, sipType_TYPE, state);
}
qm->insert(k, qm2);
#endif
}

*sipCppPtr = qm;
Expand Down

5 comments on commit fc6559a

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dakcarto should this be backported?

@dakcarto
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is now. Wanted to wait for a build or two across platforms before backporting, since I only tested on Mac.

@sebastic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll include this change in the qgis Debian package to resolve Debian Bug #822477, it's unfortunate that this change didn't make it into todays 2.14.2 release. I look forward to dropping the patch for update to 2.14.3.

@jef-n
Copy link
Member

@jef-n jef-n commented on fc6559a Apr 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about 8dcd918?

@sebastic
Copy link
Contributor

@sebastic sebastic commented on fc6559a Apr 29, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.