Vitezslav Samel <vitezslav(a)samel.cz> writes:
On Thu, May 29, 2014 at 02:22:21AM +1200, Nikola Pajkovsky wrote:
> Vitezslav Samel <vitezslav(a)samel.cz> writes:
>
> > Don't bother about printed entry pointer correctness in main loop
> > but check for NULL in print_serv_rates() function instead.
> >
> > Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
> > ---
> > src/serv.c | 40 +++++++++++++++++++++-------------------
> > 1 file changed, 21 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/serv.c b/src/serv.c
> > index 0d4a057..986fc3f 100644
> > --- a/src/serv.c
> > +++ b/src/serv.c
> > @@ -687,21 +687,26 @@ static void show_portsort_keywin(WINDOW ** win, PANEL **
panel)
> >
> > static void print_serv_rates(struct portlistent *ple, WINDOW *win)
> > {
> > - char buf[64];
> > -
> > - wattrset(win, IPSTATLABELATTR);
> > - mvwprintw(win, 0, 1, "Protocol data rates:");
> > - mvwprintw(win, 0, 36, "total");
> > - mvwprintw(win, 0, 57, "in");
> > - mvwprintw(win, 0, 76, "out");
> > -
> > - wattrset(win, IPSTATATTR);
> > - rate_print(rate_get_average(&ple->rate), buf, sizeof(buf));
> > - mvwprintw(win, 0, 21, "%s", buf);
> > - rate_print(rate_get_average(&ple->rate_in), buf, sizeof(buf));
> > - mvwprintw(win, 0, 42, "%s", buf);
> > - rate_print(rate_get_average(&ple->rate_out), buf, sizeof(buf));
> > - mvwprintw(win, 0, 61, "%s", buf);
> > + if (ple == NULL) {
> > + wattrset(win, IPSTATATTR);
> > + mvwprintw(win, 0, 1, "No entries");
> > + } else {
> > + char buf[64];
> > +
> > + wattrset(win, IPSTATLABELATTR);
> > + mvwprintw(win, 0, 1, "Protocol data rates:");
> > + mvwprintw(win, 0, 36, "total");
> > + mvwprintw(win, 0, 57, "in");
> > + mvwprintw(win, 0, 76, "out");
> > +
> > + wattrset(win, IPSTATATTR);
> > + rate_print(rate_get_average(&ple->rate), buf, sizeof(buf));
> > + mvwprintw(win, 0, 21, "%s", buf);
> > + rate_print(rate_get_average(&ple->rate_in), buf, sizeof(buf));
> > + mvwprintw(win, 0, 42, "%s", buf);
> > + rate_print(rate_get_average(&ple->rate_out), buf, sizeof(buf));
> > + mvwprintw(win, 0, 61, "%s", buf);
> > + }
> > }
> >
> > static void update_serv_rates(struct portlist *list, unsigned long msecs)
> > @@ -817,8 +822,6 @@ void servmon(char *ifname, time_t facilitytime)
> > updtime = tv;
> > starttime = startlog = timeint = tv.tv_sec;
> >
> > - wattrset(statwin, IPSTATATTR);
> > - mvwprintw(statwin, 0, 1, "No entries");
> > update_panels();
> > doupdate();
> >
> > @@ -858,8 +861,7 @@ void servmon(char *ifname, time_t facilitytime)
> > update_serv_rates(&list, rate_msecs);
> >
> > /* ... and print the current one */
> > - if (list.barptr != NULL)
> > - print_serv_rates(list.barptr, statwin);
> > + print_serv_rates(list.barptr, statwin);
> >
> > dropped += packet_get_dropped(fd);
> > print_packet_drops(dropped, list.borderwin, LINES - 4, 49);
>
> I don't have problem with the code itself, but the coding style. If you
> don't mind, I will do
>
> if (!ple) {
> ...
> *return*;
> }
>
> <rest code>
But I prefer one point of exit from a function (with exception of
checking validity of passed args with no code before return).
Whole point here is, that we are trying (or at least me) to avoid that
code, to make it more liner without too much indenting, because you can
easily write code like that.
<some code here>
if (a) {
while (b) {
if (c) {
... good stuff ...
} else {
... bad stuff ....
}
}
} else {
... handle error,
set a to some reasonable value ...
}
what now if I need to use a? again if else to handle check error path?
Way better solution is to bail out as soon as possible if function
cannot reasonably continue. See the difference:
<some code here>
if (!a)
return;
from now on, var a is reasonable value and we can read/write
while (b) {
if (c) {
... good stuff ...
} else {
... bad suff ...
}
}
--
Nikola