search

neraliu / tainted-phantomjs

Tainted PhantomJS
BSD 3-Clause "New" or "Revised" License
53 stars 12 forks source link

create the version for 32 bit and 64 bit branch. #15

Closed neraliu closed 10 years ago

neraliu commented 10 years ago

as there are 2 different implementations for the 32 bit and 64 bit version of tpjs, i need to consolidate and refactor the code to make it more easy to manage, i will do the code clean up and start creating the versioning to the code.

neraliu commented 10 years ago

In order to make the code easy to manage, i will create 2 branches called 32-bit-vX.X and 64-bit-vX.X onwards, while i will use the tag to mark which branch/tag of the original phantomjs i am working on.

andresriancho commented 10 years ago

Keeping two different branches will be a nightmare for you. Each time someone fixes a bug, or adds a new feature you'll have to merge it into the other branch, etc. Can't you use some type of conditional code? If it would be python, I would do something like:

if sys.is_64bit():
    from somelib64 import foo64 as foo
else:
    from somelib import foo

foo()
neraliu commented 10 years ago

yes. your suggestion makes sense to me

yukinying commented 10 years ago

Actually it is no longer a 64 bit issue or not.. I suggest we define 2 tainting methods now:

  1. UString_Extending
  2. UString_Hashmap

Essentially [2] works for both 32 and 64 bit, while [1] works for 32 bit only. Thus making [2] as default may make more sense.

I have serious concern that [1] will not work in some edge cases due to the way they manipulate their stack storage is still a mystery to us.

Thoughts?

neraliu commented 10 years ago

agreed that hashmap should be the default implementation onwards

neraliu commented 10 years ago

we will maintain one branch with platform detection now https://github.com/neraliu/tpjs/commit/73cf816fe22af0868fc4060be5412487ac2821ad