Vitezslav Samel <vitezslav(a)samel.cz> writes:
On Mon, Mar 12, 2012 at 09:19:04AM +0100, Nikola Pajkovsky wrote:
> Vitezslav Samel <vitezslav(a)samel.cz> writes:
>
> > Make use of structures and helper functions to clean up this function.
> >
> > Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
> > ---
> > src/ifstats.c | 328 +++++++++++++++++---------------------------------------
> > src/ifstats.h | 80 ++++----------
> > src/log.c | 110 ++++++++++++--------
> > 3 files changed, 187 insertions(+), 331 deletions(-)
> >
>
> > + ts->total.total.packets,
> > + ts->total.total.bytes);
>
> This looks pretty bad. Total in total? The code will became hard to
> grep. I suggest to change the name of items in struct.
>
> counter looks to me too generic. it's packet counter. better name could
> be *pkt_counter* and all the items should be prefixed with pc_ like
>
> struct pkt_counter
DONE.
> unsigned long long pc_packets;
DONE. Greppable but ugly to my eyes.
that depends. I'm using that whenever some common name shows in the
struc like DIR* struct or stat struct
> > +struct counter {
> > + unsigned long long packets;
>
> uint64_t?
No. One big problem: what will be in printf format?
%lu (on x86_64) or %llu (on x86_32) ???
yea; then leave it or it could be used the *size_t* and printf format %z
> > + unsigned long long bytes;
> > +};
>
> prefix it with e.g. with *pr_*
DONE (I used proto_ prefix).
If attached patch suits you, I'll make it a git commit and send it to
you in proper format.
rest is good to me.
--
Nikola