search

keystone-engine / keypatch

Multi-architecture assembler for IDA Pro. Powered by Keystone Engine.
http://www.keystone-engine.org/keypatch
GNU General Public License v2.0
1.51k stars 358 forks source link

Fix keypatch for IDA7.4 compatible to both python2 and 3 #67

Closed hotwinter closed 4 years ago

hotwinter commented 5 years ago

This should work for IDA version 7.0 up to 7.4 (need to be tested), since 7.4 deprecated API before 6.95, this change is necessary for keypatch to work on versions above 7.4.

Currently this is only thoroughly tested in IDA 7.4 for both python2 and python3, will try to get more versions tested

aquynh commented 5 years ago

Pls be sure this does not break 6.x versions

Rupan commented 5 years ago

This won't work on IDA < 7 at least because the import names are different. I get that backwards compatibility is important, but consider that Python 2 is being deprecated by the Python foundation at the end of the year. Adding a dependency on six just makes the code less maintainable and is unnecessary given how long IDA 7 has been out.

hotwinter commented 5 years ago

This won't work on IDA < 7 at least because the import names are different. I get that backwards compatibility is important, but consider that Python 2 is being deprecated by the Python foundation at the end of the year. Adding a dependency on six just makes the code less maintainable and is unnecessary given how long IDA 7 has been out.

six is probably still necessary, given that even in IDA7.4 you can still select to use python2 for IDAPython, so even given the current installation, it is still needed at least for now. I used it in order to convert between unicode in python2 and bytes in python3 to string (since ctype only allows const char*), str won't work against either python2's unicode string with unicode characters or python3's byte string (would leave b' at the front). If someone has a better thought of how this can be more python23 compatible, I would be more than happy to change it

aquynh commented 5 years ago

A lot of people are still on 6.x, so backward compatible is important.

hotwinter commented 5 years ago

A lot of people are still on 6.x, so backward compatible is important.

Addressing #65, I've added the compatibility support for IDA 6.x, it is now thoroughly tested on the following versions, which are all the versions I currently have access to, if needed to test on more versions, probably need to reach out to more people

IDA7.4Sp1 Python2 and 3, IDA7.4 Python2 and 3, IDA7.1, IDA6.95, IDA6.8, IDA6.6

aquynh commented 5 years ago

@quangnh89 could you please help to test this PR?

Rupan commented 5 years ago

Wow, pretty extensive changes.

kciredor commented 4 years ago

Looking forward to this one getting merged!

aquynh commented 4 years ago

merged, thanks!

aquynh commented 4 years ago

@hotwinter please add your name to https://github.com/keystone-engine/keypatch/blob/master/CREDITS.TXT, thanks.