Skip to content

Support Multiple ifdnfc Devices With Autostart/Hotplug of pn532 (and maybe other) Serial. - #5

Open
benmehlman wants to merge 6 commits into
nfc-tools:masterfrom
benmehlman:serial-nfc-open-on-startup
Open

Support Multiple ifdnfc Devices With Autostart/Hotplug of pn532 (and maybe other) Serial.#5
benmehlman wants to merge 6 commits into
nfc-tools:masterfrom
benmehlman:serial-nfc-open-on-startup

Conversation

@benmehlman

Copy link
Copy Markdown

This is the code which I posted about on the pcsclite-muscle mailing list. It allows ifdnfc to support multiple serial devices including startup without human intervention.

Comment thread src/ifd-nfc.c Outdated
#include <inttypes.h>
#include <time.h>

extern char *strdup(char *);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should use #include <string.h> instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my surprise it turns out that strdup() is not POSIX and compiler is flagged for POSIX.. hence even when I included string.h there was no prototype for strdup(). I didn't want to remove POSIX compliance because I didn't know what platforms this code is being used on.. so I replaced my strdup with malloc+strcpy. This line of code is a leftover, I will remove it.

@LudovicRousseau

Copy link
Copy Markdown
Contributor

The patch a2aad50 is very big and mix different changes.

Maybe you could split the patch in small & independent changes?

@benmehlman

benmehlman commented Sep 1, 2016

Copy link
Copy Markdown
Author

Yes I'm sorry about the size of the last commit. The problem was that I didn't know all the work I was going to do until I started doing it. I thought that the change to ifdnfc-activate was going to be much smaller.. I ended up touching almost every line.. also did not anticipate all the other problems (memory handling issues etc) that I found along the way. It was not done in any sort of order but fixed as I went along sometimes multiple iterations of fixes.. making change 1, not working because need change 2, then need 3.. finally able to test change 1 and find out that it needs some more work.. so change again.. and so on until everything works smoothly. That this was all on Raspberry pi didn't help. So I would have to basically start over to get it into separate commits. Right now I'm under a lot of work pressure and so if you would consider the patch without splitting that would be really helpful. Otherwise it will be some time before I can do it. I will take care of the comments and the few small cleanups in the next day or two. Thanks!

@LudovicRousseau

Copy link
Copy Markdown
Contributor

One big benefit of git is that you can rewrite the history and clean up your patches.
For example you could merge (called squash in git) edcfbe0 into a2aad50 so that the declaration of strdup() never appear in your patches.

Take your time.

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.

2 participants