On Fri, 15 Apr 2011 15:09:57 +0300 Sergey Senozhatsky <[email protected]> wrote:
> device/network and tuning/ethernet had the same "/sys/class/net/" > directory traverse code with the only difference in post-actions (filling > all_tunables or nics/all_devices). > > - Remove duplicate code and introduce function callback to perform mentioned > post-actions: > > . netdev_callback -- (default) to fill nics and all_devices > . ethtunable_callback -- to fill all_tunables. > > Signature of create_all_nics has been changed accordingly to accept > `typedef void (*callback)(const char*);' and "/sys/class/net/" traverse > moved > to read_all_nics() (from original code removed unused `ifstream file', char > filename[4096] replaced with std::string later in netdev_callback). > > - network::network(char*, char*) has been changed to accept const char* > parameters (T* to const T* cast is trivial), stack usage optimized by > replacing `char filename[4096]' with std::string. > > > Signed-off-by: Sergey Senozhatsky <[email protected]> It would be easier to read if you separate out the cleanup type changes (i.e. whitespace) from the changes that need more reviewing. > > --- > > diff --git a/devices/device.h b/devices/device.h > index cc444f1..a95e281 100644 > --- a/devices/device.h > +++ b/devices/device.h > @@ -77,7 +77,7 @@ extern void report_devices(void); > extern void html_report_devices(void); > > > - > extern void create_all_devices(void); > > -#endif > \ No newline at end of file > +#endif > + Please don't add whitespace to the end of the file. > diff --git a/devices/network.cpp b/devices/network.cpp > index e350eec..181ae15 100644 > --- a/devices/network.cpp > +++ b/devices/network.cpp > @@ -107,10 +107,10 @@ static void do_proc_net_dev(void) > } > > > -network::network(char *_name, char *path): device() > +network::network(const char *_name, const char *path): device() > { > char line[4096]; > - char filename[4096]; > + std::string filename(path); > char devname[128]; > start_up = 0; > end_up = 0; > @@ -155,9 +155,9 @@ network::network(char *_name, char *path): device() > rindex_pkts = get_result_index(devname); > > memset(line, 0, 4096); > - sprintf(filename, "%s/device/driver", path); > - if (readlink(filename, line, 4096) > 0) { > - sprintf(humanname, _("Network interface: %s (%s)"),_name, > basename(line)); > + filename.append("/device/driver"); > + if (readlink(filename.c_str(), line, 4096) > 0) { > + sprintf(humanname, _("Network interface: %s (%s)"), _name, > basename(line)); > }; > } > > @@ -327,17 +327,15 @@ const char * network::device_name(void) > return name; > } > > -void create_all_nics(void) > + > +void read_all_nics(callback fn) Please don't add extra whitespace, only one space between functions. I know it isn't consistent in the current code, but we can start being consistent now. > { > struct dirent *entry; > DIR *dir; > - char filename[4096]; > dir = opendir("/sys/class/net/"); > if (!dir) > return; > while (1) { > - class network *bl; > - ifstream file; > entry = readdir(dir); > if (!entry) > break; > @@ -346,16 +344,33 @@ void create_all_nics(void) > if (strcmp(entry->d_name, "lo")==0) > continue; > > - sprintf(filename, "/sys/class/net/%s", entry->d_name); > - bl = new class network(entry->d_name, filename); > - all_devices.push_back(bl); > - nics[entry->d_name] = bl; > + fn(entry->d_name); > } > + > closedir(dir); > +} > > + > +void netdev_callback(const char *d_name) extra whitespace. > +{ > + std::string f_name("/sys/class/net/"); > + f_name.append(d_name); > + > + network *bl = new(std::nothrow) class network(d_name, f_name.c_str()); > + if (bl) { > + all_devices.push_back(bl); > + nics[d_name] = bl; > + } > } > > > +void create_all_nics(callback fn) > +{ > + if (!fn) > + fn = &netdev_callback; > + read_all_nics(fn); > +} > + > > double network::power_usage(struct result_bundle *result, struct > parameter_bundle *bundle) > { > @@ -369,8 +384,6 @@ double network::power_usage(struct result_bundle *result, > struct parameter_bundl > > power += util * factor; > > - > - > if (valid_100 == -1) { > valid_100 = utilization_power_valid(rindex_link_100); > valid_1000 = utilization_power_valid(rindex_link_1000); > @@ -411,4 +424,5 @@ double network::power_usage(struct result_bundle *result, > struct parameter_bundl > power += util * factor / 100; > > return power; > -} > \ No newline at end of file > +} > + extra whitespace > diff --git a/devices/network.h b/devices/network.h > index 50bf61e..76e17ac 100644 > --- a/devices/network.h > +++ b/devices/network.h > @@ -62,7 +62,7 @@ public: > uint64_t pkts; > double duration; > > - network(char *_name, char *path); > + network(const char *_name, const char *path); > > virtual void start_measurement(void); > virtual void end_measurement(void); > @@ -79,7 +79,7 @@ public: > virtual int grouping_prio(void) { return 10; }; > }; > > -extern void create_all_nics(void); > +extern void create_all_nics(callback fn = NULL); > > +#endif > > -#endif > \ No newline at end of file > diff --git a/lib.h b/lib.h > index f6f924f..3d195d4 100644 > --- a/lib.h > +++ b/lib.h > @@ -71,6 +71,8 @@ extern char *fmt_prefix(double n, char *buf); > extern char *pretty_print(const char *str, char *buf, int len); > extern int equals(double a, double b); > > +typedef void (*callback)(const char*); > extern int utf_ok; > + > #endif > > diff --git a/tuning/ethernet.cpp b/tuning/ethernet.cpp > index f13f720..4666ac1 100644 > --- a/tuning/ethernet.cpp > +++ b/tuning/ethernet.cpp > @@ -47,6 +47,8 @@ > #include "../lib.h" > #include "ethernet.h" > > +extern void create_all_nics(callback fn); > + > ethernet_tunable::ethernet_tunable(const char *iface) : tunable("", 0.3, > _("Good"), _("Bad"), _("Unknown")) > { > memset(interf, 0, sizeof(interf)); > @@ -127,29 +129,15 @@ void ethernet_tunable::toggle(void) > } > > > - > +void ethtunable_callback(const char *d_name) > +{ > + class ethernet_tunable *eth = new(std::nothrow) class > ethernet_tunable(d_name); > + if (eth) > + all_tunables.push_back(eth); > +} > > void add_ethernet_tunable(void) > { > - class ethernet_tunable *eth; > - struct dirent *entry; > - DIR *dir; > - dir = opendir("/sys/class/net/"); > - if (!dir) > - return; > - while ((entry = readdir(dir))) { > - if (!entry) > - break; > - if (entry->d_name[0] == '.') > - continue; > - if (strcmp(entry->d_name, "lo") == 0) > - continue; > - > - eth = new(std::nothrow) class ethernet_tunable(entry->d_name); > - if (eth) > - all_tunables.push_back(eth); > - } > - > - closedir(dir); > + create_all_nics(ðtunable_callback); > } > > diff --git a/tuning/ethernet.h b/tuning/ethernet.h > index 8050aca..1c57f21 100644 > --- a/tuning/ethernet.h > +++ b/tuning/ethernet.h > @@ -44,5 +44,5 @@ public: > > extern void add_ethernet_tunable(void); > > - > #endif > + random extra whitespace _______________________________________________ Discuss mailing list [email protected] http://lists.lesswatts.org/listinfo/discuss
