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

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

benmehlman
Copy link

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.

@@ -102,6 +103,8 @@ void log_msg(const int priority, const char *fmt, ...)
#include <inttypes.h>
#include <time.h>

extern char *strdup(char *);
Copy link
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
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
Contributor

The patch a2aad50 is very big and mix different changes.

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

@benmehlman
Copy link
Author

benmehlman commented Sep 1, 2016

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
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