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

Maybe mistyping =+ in your source code #6

Open
ViaExplore opened this issue Jun 14, 2017 · 6 comments
Open

Maybe mistyping =+ in your source code #6

ViaExplore opened this issue Jun 14, 2017 · 6 comments
Labels

Comments

@ViaExplore
Copy link

Hi @cederom

I using your lib in one of my project and found maybe a mistyping in your source code.
For example in file libswd_cmdq.c line 243:

res=libswd_drv_transmit(libswdctx, cmd);
if (res<0) return res;
cmdcnt=+res;

the cmdcnt will then hold only the last result of res not the sum of them, which will emit in some cases problems.
To fix all these kind of problems, maybe it would be better to find all "=+" across the libswd source code and change it to "+="

Best regards,
Tomas Kamenicky

@cederom
Copy link
Owner

cederom commented Jun 14, 2017

Good point @MemphisCZ! Thanks! Will check! :-)

@ViaExplore
Copy link
Author

One more suggestion about fflush(0);, in some multitask FREERTOS application this may cause task dead lock, flushing NULL pointer means flush everything. In such OS some kind of opened file descriptors could be just input streams waiting for input data and already locked with their recursive lock. Flush from different task will be waiting for ever until first task will receive something and releases the lock. Would be better to use fflush(stdout); in libswd_memap.c line 270,314,470,505,683,724,875,909. :-]

@cederom
Copy link
Owner

cederom commented Jun 14, 2017

Thanks for hint @MemphisCZ! Patches welcome! :-)
You can simply fork a project here on GitHub, then make local changes, commit them, and make a PULL REQUEST to send changes to the upstream project, piece of cake :-)

@cederom
Copy link
Owner

cederom commented Jun 14, 2017

Also I think this fflush() should be moved to libswd_log() as it has nothing to do with MEMAP itself..? :-)

@ViaExplore
Copy link
Author

flushing each time for log output will drastically slow down the logging. Stdout for frequent flush slow as hell, as it will call OS kernel rutins. Would be better to delete it from library as user of that library can set up his stdout with something like:
setvbuf(stdout, NULL, _IONBF, 0);
this will ensure that all log messages will be outputted immediately if he needs this. Or he can use fflush himself in app region.

...and one more question about memory consumption. Where are the points in source code for cmdq memory release. I found function libswd_cmdq_free_head, but it is nowhere called, so after few rutins it eats all my 64k embedded memory :-D How can I find the right moment for cmdq release?

@cederom
Copy link
Owner

cederom commented Jun 14, 2017

Or we can simply create additional libswd_log_flush() that would call libswd_log() and fflush().. from what I remember that was necessary to see the progress updates on screen..

Regarding the queue flush, this still need to be done in the queue handler after hitting the LIBSWD_CMDQLEN_DEFAULT :-P Had no need for that, but stuff is there to be used and verified, feel free to make it work as you already have the environment for testing :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants