LibSWD icon indicating copy to clipboard operation
LibSWD copied to clipboard

Maybe mistyping =+ in your source code

Open ViaExplore opened this issue 8 years ago • 6 comments

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

ViaExplore avatar Jun 14 '17 10:06 ViaExplore

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

cederom avatar Jun 14 '17 11:06 cederom

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. :-]

ViaExplore avatar Jun 14 '17 12:06 ViaExplore

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 avatar Jun 14 '17 12:06 cederom

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

cederom avatar Jun 14 '17 12:06 cederom

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?

ViaExplore avatar Jun 14 '17 12:06 ViaExplore

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

cederom avatar Jun 14 '17 12:06 cederom