From 32bff14871bf8f534fcdcdba1d8409430d6c464b Mon Sep 17 00:00:00 2001 From: David Phillips Date: Tue, 1 Sep 2015 21:19:01 +1200 Subject: Conforming to style, switching paramano-set to getopt (finally) --- TODO.md | 1 - bat_tray.c | 20 +++---- common.c | 13 +---- common.h | 6 +- getcore.h | 2 +- getfreq.c | 37 ++++++------ getfreq.h | 3 +- getgov.c | 22 ++++--- getgov.h | 2 +- paramano.c | 22 +++---- paramano.h | 2 +- paramano_set.c | 149 ++++++++++++++++++++--------------------------- paramano_set_interface.h | 2 +- tray.c | 17 +++--- tray.h | 2 +- 15 files changed, 130 insertions(+), 170 deletions(-) diff --git a/TODO.md b/TODO.md index b0926ac..ad74478 100644 --- a/TODO.md +++ b/TODO.md @@ -8,5 +8,4 @@ Normal Priority Low Priority ------------ -* Ensure all code aligns to an explicit style * Make a curses-based version of paramano diff --git a/bat_tray.c b/bat_tray.c index a848278..446dcf4 100644 --- a/bat_tray.c +++ b/bat_tray.c @@ -43,7 +43,6 @@ long get_bat_seconds_left() int full = 0; int rate = 0; - /* FIXME this now/full/rate logic has probably earned its own function. * Not because of DRY, but just for readability's sake perhaps */ @@ -81,12 +80,8 @@ long get_bat_seconds_left() /* Note the '<=' for rate (we divide by rate) */ - if (full < 0 || - now < 0 || - rate <= 0) - { + if (full < 0 || now < 0 || rate <= 0) return -1; - } switch(get_battery_state()) { @@ -188,10 +183,10 @@ void bat_tray_init() { char icon_file[1024]; - // Get the battery number, store it for later + /* Get the battery number, store it for later */ bat_num = get_bat_num(); - // Set up battery info filenames/paths + /* Set up battery info filenames/paths */ snprintf(CHARGE_VALUE_PATH, sizeof(CHARGE_STATE_PATH), "/sys/class/power_supply/BAT%d/capacity", bat_num); snprintf(CHARGE_STATE_PATH, sizeof(CHARGE_STATE_PATH), "/sys/class/power_supply/BAT%d/status", bat_num); @@ -231,7 +226,7 @@ void bat_tray_hide() int get_battery_state() { char state[1024]; - FILE* fd; + FILE* fd = NULL; if (!(fd = fopen(CHARGE_STATE_PATH, "r")) || !fgets(state, sizeof(state), fd)) @@ -260,13 +255,14 @@ int get_battery_state() **********************************************************************/ int get_bat_num() { - FILE* fd; + FILE* fd = NULL; char filename[1024]; - unsigned int i; + int i = 0; + for(i = 0; i < 10; i++) { snprintf(filename, sizeof(filename), "/sys/class/power_supply/BAT%d/present", i); - if( (fd = fopen(filename, "r")) ) + if((fd = fopen(filename, "r"))) { if (fgetc(fd) == '1') { diff --git a/common.c b/common.c index d1be19b..953a33b 100644 --- a/common.c +++ b/common.c @@ -30,7 +30,6 @@ int get_int_value_from_filef(const char* format, ...) va_start(a, format); value = vget_int_value_from_filef(format, a); va_end(a); - return value; } @@ -40,7 +39,6 @@ int get_int_value_from_filef(const char* format, ...) **********************************************************************/ int vget_int_value_from_filef(const char* format, va_list args) { - char filename[1024]; if (vsnprintf(filename, sizeof(filename), format, args) == sizeof(filename)) fprintf(stderr, "WARN: filename buffer too small"); @@ -53,9 +51,9 @@ int vget_int_value_from_filef(const char* format, va_list args) **********************************************************************/ int get_int_value_from_file(const char* filename) { - FILE* fd; + FILE* fd = NULL; char buffer[512]; - int value; + int value = 0; if(!(fd = fopen(filename, "r"))) return -1; @@ -68,7 +66,6 @@ int get_int_value_from_file(const char* filename) } - /*********************************************************************** * Truncates a string at the first '\r' or '\n' **********************************************************************/ @@ -84,9 +81,5 @@ void chomp(char *string) **********************************************************************/ int get_int(const char* string) { - char* first_num; - - first_num = strpbrk(string, "0123456789"); - - return atoi(first_num); + return atoi(strpbrk(string, "0123456789")); } diff --git a/common.h b/common.h index 24f9dd6..b672e0d 100644 --- a/common.h +++ b/common.h @@ -27,12 +27,12 @@ int get_int_value_from_file(const char* filename); void chomp(char *string); int get_int(const char* string); -// Stringification of line number +/* Stringification of line number */ #define STRING2(x) #x #define STRING(x) STRING2(x) #define STR_LINE STRING(__LINE__) -// +/* */ #define info(...) printf("INFO: "__FILE__":"STR_LINE" --- "__VA_ARGS__) #define FILE_PATH_SIZE 2048 -#endif +#endif /* ifndef COMMON_H */ diff --git a/getcore.h b/getcore.h index 5e6cbb9..ecdca9d 100644 --- a/getcore.h +++ b/getcore.h @@ -22,4 +22,4 @@ void gc_init(); unsigned int gc_number(); -#endif +#endif /* ifndef GETCORE_H */ diff --git a/getfreq.c b/getfreq.c index fbf46e9..3ff6245 100644 --- a/getfreq.c +++ b/getfreq.c @@ -23,7 +23,7 @@ #define FREQ_LENGTH 14 static char freqs[MAX_CORES][MAX_FREQS][FREQ_LENGTH]; -static int total_freqs; // Number of freqs for core 0 +static int total_freqs; /* Number of freqs for core 0 */ /*********************************************************************** @@ -31,25 +31,24 @@ static int total_freqs; // Number of freqs for core 0 **********************************************************************/ void gf_init() { - char freq_string[4096]; // POSIX suggested line length. Source of error for CPUs with a huge number of freqs - char *next_token; - unsigned int i; + char freq_string[4096]; /* POSIX suggested line length. Source of error for CPUs with a huge number of freqs */ + char *next_token = NULL; + unsigned int i = 0; memset(freqs, '\0', sizeof(freqs)); - - for(i = 0; (i < gc_number() && i < MAX_CORES); i++) + for(i = 0; i < gc_number() && i < MAX_CORES; i++) { memset(freq_string, '\0', sizeof(freq_string) ); - // Get available governor freqs. If no governor, try next cpu + /* Get available governor freqs. If no governor, try next cpu */ if (gf_available(i, freq_string, sizeof(freq_string) ) == -1) continue; *strchrnul(freq_string, '\n') = '\0'; - // freq_string is a space separated list of freqs - // Use strtok to find each + /* freq_string is a space separated list of freqs. + * Use strtok to find each */ next_token = strtok(freq_string, " \n"); total_freqs = 0; do @@ -59,7 +58,7 @@ void gf_init() } while((next_token = strtok(NULL, " ")) != NULL); } - // Hit the limit of storage of cores' frequencies + /* Hit the limit of storage of cores' frequencies */ if (i == MAX_CORES) info("Unable to add more than %d cores\n", MAX_CORES); } @@ -69,18 +68,17 @@ void gf_init() **********************************************************************/ int gf_current(int core) { - FILE* fd; + FILE* fd = NULL; char buff[4096]; char path[1024]; - int freq; + int freq = 0; snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq", core); if(!(fd = fopen(path, "r"))) return -1; - fgets(buff, 13, fd); - + fgets(buff, 13, fd); /* FIXME magic */ freq = atoi(buff); fclose(fd); @@ -93,7 +91,7 @@ int gf_current(int core) **********************************************************************/ int gf_available(int core, char* out, int size) { - FILE* fd; + FILE* fd = NULL; char path[1024]; snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_available_frequencies", core); @@ -102,7 +100,6 @@ int gf_available(int core, char* out, int size) return -1; fgets(out, size, fd); - fclose(fd); return 0; } @@ -112,13 +109,13 @@ int gf_available(int core, char* out, int size) **********************************************************************/ void gf_get_frequency_label(char *buffer, size_t max_size, int freq) { - if(freq >= 1000000000) // >= 1 billion KHz (1 THz) This, ladies and gentlement, is future-proofing ;) + if(freq >= 1000000000) /* This, ladies and gentlement, is future-proofing */ snprintf(buffer, max_size, "%.2f THz", (double)freq/1000000000 ); - else if(freq >= 1000000) // >= 1 million KHz (1 GHz) + else if(freq >= 1000000) snprintf(buffer, max_size, "%.2f GHz", (double)freq/1000000 ); - else if (freq >= 1000) // >= 1 thousand KHz (1 MHz) + else if (freq >= 1000) snprintf(buffer, max_size, "%.2f MHz", (double)freq/1000 ); - else // < 1000 KHz (1 MHz) + else snprintf(buffer, max_size, "%.2f KHz", (double)freq); } diff --git a/getfreq.h b/getfreq.h index f3c490c..5ec9d35 100644 --- a/getfreq.h +++ b/getfreq.h @@ -27,5 +27,4 @@ char* gf_freqa(int core, int index); int gf_freqi(int core, int index); unsigned int gf_number(); - -#endif +#endif /* ifndef GETFREQ_H */ diff --git a/getgov.c b/getgov.c index 7ff8562..f75e6c9 100644 --- a/getgov.c +++ b/getgov.c @@ -27,20 +27,20 @@ static int total_governors; **********************************************************************/ void gg_init() { - gchar gov_string[500]; + char gov_string[500]; unsigned int i = 0; total_governors = 0; + for (i = 0; i < gc_number(); i++) { memset(gov_string, '\0', sizeof(gov_string) ); gg_available(i, gov_string, sizeof(gov_string) ); - // go through every governor in gov_string - gchar* curr = (char*)&gov_string; - gchar* end_of_curr = g_strstr_len(curr, strlen(curr), " "); + char* curr = gov_string; + char* end_of_curr = g_strstr_len(curr, strlen(curr), " "); while (end_of_curr) { - memset(governors[i][total_governors], '\0', 13); + memset(governors[i][total_governors], '\0', 13); /* FIXME magic */ memmove(governors[i][total_governors], curr, end_of_curr - curr); curr = end_of_curr+1; @@ -56,20 +56,17 @@ void gg_init() **********************************************************************/ bool gg_current(int core, char* out, int size) { - FILE* fd; + FILE* fd = NULL; char path[1024]; + snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", core); if (!(fd = fopen(path, "r"))) return false; fgets(out, size, fd); - - // Get first line - gchar* newline = g_strrstr(out, "\n"); - *newline = '\0'; - fclose(fd); + chomp(out); return true; } @@ -79,8 +76,9 @@ bool gg_current(int core, char* out, int size) **********************************************************************/ bool gg_available(int core, char* out, int size) { + FILE *fd = NULL; char path[1024]; - FILE *fd; + snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_available_governors", core); if (!(fd = fopen(path, "r"))) diff --git a/getgov.h b/getgov.h index 131f164..9339d20 100644 --- a/getgov.h +++ b/getgov.h @@ -25,4 +25,4 @@ bool gg_available(int core, char* out, int size); char* gg_gov(int core, int index); unsigned int gg_number(); -#endif +#endif /* ifndef GETGOV_H */ diff --git a/paramano.c b/paramano.c index b0e2671..1af70a7 100644 --- a/paramano.c +++ b/paramano.c @@ -41,11 +41,11 @@ int main(int argc, char** argv) tray_init(); tray_show(); - // Show battery tray only if we're supposed to + /* Show battery tray only if we're supposed to */ if(DEFAULT_SHOW_BATTERY) { bat_tray_init(); - bat_tray_show(); +// bat_tray_show(); } gtk_main(); @@ -60,22 +60,22 @@ void config_init() { struct config_file config; FILE* fd = NULL; - gboolean home_config_exists; - config.key_file = NULL; + bool home_config_exists = false; + config.key_file = NULL; config.file_name = g_strconcat(getenv("HOME"), "/.paramano.conf", NULL); - // Check if ~/.paramano.conf exists + /* Check if ~/.paramano.conf exists */ if( (fd = fopen(config.file_name, "r")) ) { - // If file exists, close it, set param to TRUE + /* If file exists, close it, set param to true */ fclose(fd); - home_config_exists = TRUE; + home_config_exists = true; } else { - // If file didn't exist, it's not open (don't close it), free filename var, set param to FALSE + /* If file didn't exist, it's not open (don't close it), free filename var, set param to false */ g_free(config.file_name); - home_config_exists = FALSE; + home_config_exists = false; } if(!home_config_exists) @@ -88,7 +88,7 @@ void config_init() return; } - // Reset defaults to default values + /* Reset defaults to default values */ defaults_init(); DEFAULT_GOV = config_get_key(&config, "governor", "default"); @@ -105,7 +105,7 @@ void config_init() if ((temp = config_get_key(&config, "extra", "theme"))) snprintf(DEFAULT_THEME, sizeof(DEFAULT_THEME), "%s", temp); - g_free(config.file_name); +// g_free(config.file_name); config_close(&config); } diff --git a/paramano.h b/paramano.h index 430226a..9686a2e 100644 --- a/paramano.h +++ b/paramano.h @@ -42,4 +42,4 @@ void config_init(); -#endif +#endif /* ifndef PARAMANO_H */ diff --git a/paramano_set.c b/paramano_set.c index 9bcbbff..b902a84 100644 --- a/paramano_set.c +++ b/paramano_set.c @@ -19,107 +19,86 @@ #include "paramano.h" -#define FILE_PATH_STRING_SIZE 100 - -#define ARG_CORE 0x1 -#define ARG_GOV 0x2 -#define ARG_FREQ 0x4 -typedef struct { - char present; - char *core; - char *governor; - char *frequency; -} argument_summary; - -char write_str_to_file(const char *file, const char *data, const char *core) +#define set_freq_max(freq, core) write_str_to_file("scaling_max_freq", freq, core) +#define set_freq_min(freq, core) write_str_to_file("scaling_min_freq", freq, core) +#define set_speed(freq, core) write_str_to_file("scaling_setspeed", freq, core) +#define set_gov(gov, core) write_str_to_file("scaling_governor", gov, core) + +void die_syntax(const char *prefix); +char write_str_to_file(const char *file, const char *data, int core); + +int main(int argc, char *argv[]) { - FILE *fd; - char file_path[FILE_PATH_STRING_SIZE]; + char c = 0; + int core = -1; + char *frequency = NULL; + char *governor = NULL; - // Prepare file path - memset(file_path, '\0', sizeof(file_path)); - sprintf(file_path, "/sys/devices/system/cpu/cpu%d/cpufreq/%s", atoi(core), file ); + setlocale(LC_ALL, ""); + bindtextdomain("paramano", LOCALEDIR); + textdomain("paramano"); - // Try to open file and write data to it - if ( (fd = fopen(file_path, "w")) != NULL ) + if (getuid() != 0) + fprintf(stderr, "Warning: running as UID %d, not 0\n", getuid()); + + while ((c = getopt(argc, argv, "c:f:g:")) != -1) { - fputs(data, fd); - fclose(fd); - return 1; + switch (c) + { + case 'c': + core = atoi(optarg); + break; + case 'f': + frequency = optarg; + break; + case 'g': + governor = optarg; + break; + default: + die_syntax(argv[0]); + break; + } } - // Fallthrough: File couldn't be opened for writing - fprintf(stderr, _("FAILED: Couldn't open %s for writing\n") , file_path); - return 0; -} + if (core < 0 || + (governor == NULL && frequency == NULL) || + (governor != NULL && frequency != NULL)) + die_syntax(argv[0]); + if (governor != NULL) + return set_gov(governor, core); -#define set_freq_max(freq,core) write_str_to_file("scaling_max_freq",freq,core) -#define set_freq_min(freq,core) write_str_to_file("scaling_min_freq",freq,core) -#define set_speed(freq,core) write_str_to_file("scaling_setspeed",freq,core) -#define set_gov(gov,core) write_str_to_file("scaling_governor",gov,core) + if (frequency != NULL) + return set_gov("userspace", core) | set_speed(frequency, core); -void get_argument_summary(int argc, char **argv, argument_summary *argsum) -{ - int arg; + return 1; +} - // Check for -{c,g,f} xyz - for ( arg = 1; arg < argc-1; arg+=2 ) - { - // Can't have empty argument - if ( strlen(argv[arg+1]) != 0 ) - { - if ( strcmp(argv[arg], "-c") == 0 ) - { - // Found -c with an arg - argsum->present |= ARG_CORE; - argsum->core = (char*)(argv[arg+1]); - continue; - } - - if ( strcmp(argv[arg], "-f") == 0 ) - { - // Found -f with an arg - argsum->present |= ARG_FREQ; - argsum->frequency = (char*)(argv[arg+1]); - continue; - } - - if ( strcmp(argv[arg], "-g") == 0 ) - { - // Found -g with an arg - argsum->present |= ARG_GOV; - argsum->governor = (char*)(argv[arg+1]); - //continue; - } - } - } +void die_syntax(const char *prefix) +{ + fprintf(stderr, _("%s {-f frequency|-g governor} -c core\n"), prefix); + exit(1); } -int main(int argc, char *argv[]) +char write_str_to_file(const char *file, const char *data, int core) { - setlocale(LC_ALL,""); - bindtextdomain("paramano",LOCALEDIR); - textdomain("paramano"); + FILE *fd = NULL; + char file_path[1024]; - argument_summary args; - memset(&args, 0, sizeof(args)); + /* Prepare file path */ + snprintf(file_path, sizeof(file_path), "/sys/devices/system/cpu/cpu%d/cpufreq/%s", core, file); - // If unusual number of args, give up now - if (argc == 5) + /* Try to open file and write data to it */ + if ( (fd = fopen(file_path, "w")) != NULL ) { - if (getuid() != 0) - fprintf(stderr,"Warning: running as UID %d, not 0\n",getuid() ); + fputs(data, fd); + fclose(fd); + return 1; + } - get_argument_summary(argc, argv, &args); + /* Fallthrough: File couldn't be opened for writing */ + fprintf(stderr, _("FAILED: Couldn't open %s for writing\n") , file_path); + return 0; +} - if ( args.present == ( ARG_CORE | ARG_GOV ) ) - return set_gov(args.governor , args.core); - if ( args.present == ( ARG_CORE | ARG_FREQ ) ) - return set_gov("userspace", args.core) | set_speed(args.frequency, args.core); - } - // Fall through to here if no valid argument combination - fprintf(stderr, _("%s {-f frequency|-g governor} -c core\n"), argv[0] ); - return 1; -} diff --git a/paramano_set_interface.h b/paramano_set_interface.h index 20b96bf..d533285 100644 --- a/paramano_set_interface.h +++ b/paramano_set_interface.h @@ -22,4 +22,4 @@ void si_gov(char* gov, int core); void si_freq(int freq, int core); -#endif +#endif /* ifndef PARAMANO_SET_INTERFACE_H */ diff --git a/tray.c b/tray.c index a8a0b54..dafda1f 100644 --- a/tray.c +++ b/tray.c @@ -102,7 +102,7 @@ static void tray_generate_menu() gint current_frequency = gf_current(0); - // Add available frequencies + /* Add available frequencies */ for(i = 0; i < gf_number(); i++) { gf_get_frequency_label(label, sizeof(label), gf_freqi(0, i)); @@ -119,11 +119,11 @@ static void tray_generate_menu() gtk_menu_shell_append(GTK_MENU_SHELL(menu), item); } - // Add a seperator + /* Add a seperator */ GtkWidget* seperator = gtk_separator_menu_item_new(); gtk_menu_append(menu, seperator); - // Add available governors + /* Add available governors */ for(i = 0; i < gg_number(); i++) { if(g_strcmp0(gg_gov(0, i), "userspace") == 0) @@ -158,7 +158,7 @@ static gboolean show_tooltip(GtkStatusIcon* status_icon, gint x, gint y, gboolea static void update_tooltip_cache() { char msg[10240], label[1024]; - char current_governor[20]; // TO DO + char current_governor[20]; /* FIXME magic */ unsigned int i = 0; unsigned int offset = 0; @@ -198,14 +198,14 @@ static void update_icon() int max_frequency = gf_freqi(0, 0); int adjusted_percent, percent; - // If max_frequency is 0, we don't want to divide by it, - // so give up, call it a day, and have a simple icon + /* If max_frequency is 0, we don't want to divide by it, + * so give up, call it a day, and have a simple icon */ if (max_frequency == 0) { adjusted_percent = 0; } else { - // Percentages need to be {25,50,75,100}. Round to one of these numbers. - // TO DO: round/truncate instead of lots of ifs + /* Percentages need to be {25,50,75,100}. Round to one of these numbers. + * TO DO: round/truncate instead of lots of ifs */ percent = (gf_current(0) * 100)/max_frequency; if(percent == 100) { adjusted_percent = 100; @@ -264,7 +264,6 @@ static gboolean update() **********************************************************************/ void tray_set_defaults() { - // Set defaults unsigned int i = 0; if(DEFAULT_GOV) { diff --git a/tray.h b/tray.h index dfc21b8..8560017 100644 --- a/tray.h +++ b/tray.h @@ -26,4 +26,4 @@ void tray_hide(); bool tray_visible(); bool tray_embedded(); -#endif +#endif /* ifndef TRAY_H */ -- cgit v1.1