Re: [PATCH 0 of 6 V1] Counters

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

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