search

nfc-tools / libfreefare

A convenience API for NFC cards manipulations on top of libnfc.
Other
395 stars 106 forks source link

Replaced DESFIRE_TRANSCEIVE by a function (TODO n°3) #105

Closed broth-itk closed 4 years ago

broth-itk commented 5 years ago

Next try without mixing up my branches.

Replaced DESFIRE_TRANSCEIVE by a function, updating all commands it used.

smortex commented 5 years ago

You have not authorized maintainers to push changes to your branch.

Can you configure with ./configure CFLAGS="-Wall -Wextra -Werror -Wno-unused-variable -Wno-unused-function" and fix the problems detected (a cast missing, and a couple of char * that should by uint8_t *?

Thanks!

I also feel that when compiling WITH_DEBUG, MIFARE_DESFIRE_TRANSCEIVE() is broken.

broth-itk commented 5 years ago

You have not authorized maintainers to push changes to your branch.

I added you as collaborator - hope this solves the problem :)

Can you configure with ./configure CFLAGS="-Wall -Wextra -Werror -Wno-unused-variable -Wno->unused-function" and fix the problems detected (a cast missing, and a couple of char * that should by >uint8_t *?

Done, fixed similar issues with other parts of the lib as well. BTW: Enabling "-Werror" on configure will fail and tell that openssl headers were missing.

I also feel that when compiling WITH_DEBUG, MIFARE_DESFIRE_TRANSCEIVE() is broken.

"./configure --enable-debug" works just fine for me, no problems whatsoever

broth-itk commented 5 years ago

Travis CI fails for missing nfc-utils header file. I did not touch that. Any idea what possibly can cause that problem?

broth-itk commented 5 years ago

Is there anything left for me to do in order to get this pull request passing all checks and get eventually merged? Sorry for asking possibly dumb questions, the git process is still new to me. Thanks!

AdamLaurie commented 5 years ago

test$ ./run-test.sh make: *** No rule to make target 'echo-cutter'. Stop. ./run-test.sh: 17: ./run-test.sh: : Permission denied

don't have time to dig into this but if someone has a quick fix let me know and i'll try again...

smortex commented 5 years ago

@AdamLaurie if I recall correctly, this is the typical failure when cutter is not installed when the autotools where run.

Double-check that cutter is installed, and re-run autoreconf -is, then ./configure and make check with a formatted cards on NFC devices connected to the system.

Thanks!

darconeous commented 4 years ago

I'll give testing this a shot tonight.

darconeous commented 4 years ago

I've confirmed this change passes all tests with a MIFARE DESFire EV1.

I've also noticed that some tests fail with a MIFARE DESFire EV2 (filed #120), but I don't think that is a regression or related to this issue.

I know @smortex previously approved this change, but it looks like my conflict fix removed his approval.

smortex commented 4 years ago

Ah, indentation looks weird (maybe we should add a test for this in Travis-CI?).

I think you should be able to approve other people Pull Requests and merge them, but maybe if you contributed some code you cannot anymore?

Would you mind checking make style? Thanks!