Hi Audrius, Please accept apologies, I've been so slow at reviewing your patches. Thanks first for your contribution. I would like the following changes: - Include files: please do not add an include file for a public API. I think all of counters.h contains public data structures and function declarations that can go in libssh.h. Please remove counters.h from the cmake install script.
- Doxygen comments: You comments are great. Could you put them in the implementation of functions (counters.c) as we do for the other includes in libssh.h ? That would be a little more coherent. - Copyrights: you copy&pasted copyright headers from other files without changing them with your name. You deserve credit for your code. On the code itself: - I think you can merge ssh_bytes_counter_struct and ssh_packet_counter_struct. It's very likely that both will be used at same time and that reduces the overall complexity. ssh_counter_struct is also shorter :) Also means less setters. What do you think ? - code style: please replace if (session->packet_counter) with if (session->packet_counter != NULL) where possible. Feel free to rebase the whole diffs in a single feature patch, splitting is not required if there is a coherent whole feature patch. Msg me if you have any question Thanks, Aris Le 15/01/14 17:43, Audrius Butkevicius a écrit : > > include/libssh/counters.h | 63 > +++++++++++++++++++++++++++++++++++++++++++ > src/counters.c | 24 ++++++++++++++++ > include/libssh/CMakeLists.txt | 1 + > src/CMakeLists.txt | 1 + > include/libssh/counters.h | 23 +++++++++++++++ > include/libssh/session.h | 4 ++ > src/counters.c | 8 +++++ > src/socket.c | 4 ++ > include/libssh/counters.h | 13 +++++++- > include/libssh/session.h | 1 + > src/counters.c | 4 ++- > src/packet.c | 4 ++ > include/libssh/counters.h | 11 ++++++- > include/libssh/session.h | 1 + > src/counters.c | 2 + > src/packet.c | 4 ++ > include/libssh/channels.h | 3 ++ > include/libssh/counters.h | 23 +++++++++++++++ > src/channels.c | 6 ++++ > src/channels1.c | 2 + > src/counters.c | 9 ++++++ > 21 files changed, 207 insertions(+), 4 deletions(-) >
