RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3
http://www.icedev.se
GNU General Public License v3.0
3.62k stars 979 forks source link

Mostly implement the Operation Buffer #2353

Closed jlitewski closed 2 months ago

jlitewski commented 2 months ago

This should get 90%+ of all the cases that modify data directly to the graph buffer, and instead modify the data to the Operation Buffer instead.

This looks like a big change, but it's basically moving direct GraphBuffer writes and reads to functions I implemented to handle writing to or reading from either the Graph Buffer or the Operation Buffer and keeping those two buffers in sync.

I'm sure I missed a few, and I know I didn't touch some of the direct Graph Buffer modifications because the code looked scary and I didn't want to break something super important without having a solid way to test it locally. Most of the commands I could test all seem to output the same results as before these modifications (minus the obviously broken things with trimming and hw tune), so I count that as a win in my book.

Also felt like this was a big enough change to add to the Changelog, so I did.

jlitewski commented 2 months ago

So apparently I broke things with this. Going to go through and attempt to fix things so everything is happy.

iceman1001 commented 2 months ago

ok,

Interesting approach but it will not do. If you going for an operations buffer pattern, then you need to understand how to correctly use it in the source. Right now you merging it together and it is mess.

Rethink, understand, make a better solution

jlitewski commented 2 months ago

Okay, pushed the first revision.

I removed the get_graph_value_at() function, and I followed your advice and implemented temporary buffers to copy into and write out from in places where it was needed. I mostly just swapped out the function for writing directly to the Operation Buffer, and added text to the commands that use it warning that it does, since it is kinda a new thing

jlitewski commented 2 months ago

Okay, I'm going to close this and redo it, because somehow updating my local's master messed up this branch.