Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correcting a bug introduced in fbdbe6eff32ae5c01a6ea707a4e908c0cb1a4ebe #586

Closed
wants to merge 1 commit into from

Conversation

nmalzieu
Copy link

@nmalzieu nmalzieu commented Mar 5, 2020

The diff is not clear but the fix is actually really simple
In fbdbe6e#diff-6e1237eedf36b29193c30b63cf243259R449 a new check was added line 449 before printing an authentication error but the closing bracket } was not added.

The result is is_first_block is not closed and the rest of the code is not executed so only 1st blocks of each sector was written!

By checking the files directly it's really simple to see.

@nmalzieu
Copy link
Author

@quantum-x I think your commit introduced the bug, if you can have a look 😃

@quantum-x
Copy link
Contributor

Unless I'm mistaken, my commit was OK, it looks like it was the merge that caused the issue?
https://github.com/toyfoundry/libnfc/blame/c1e466d4a8900ba04d569fe7f268c1d942591679/utils/nfc-mfclassic.c#L443

@nmalzieu
Copy link
Author

@quantum-x ah, maybe! I don't have access to your link though. Sorry about that :)
Do you agree with the fix?

@AdamLaurie
Copy link
Member

@nmalzieu please confirm this is still an issue and provide test instructions to demonstrate problem - i have been unable to reproduce any block writing error. thanks!

@AdamLaurie
Copy link
Member

I believe this is now fixed. Thanks @nmalzieu for the initial report.

@AdamLaurie AdamLaurie closed this Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants