See: https://github.com/lxqt/liblxqt/pull/148 From a4671083ad7277288fa2a17b90efc11439088df2 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 16:31:15 +0200 Subject: [PATCH 1/8] lxqtbacklight: centralize fopen() and perform path handling with length checks --- .../linux_backend/driver/libbacklight_backend.c | 47 ++++++++++++++-------- .../linux_backend/driver/lxqtbacklight_backend.c | 14 ++----- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index 13689ef..4d90c20 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -55,6 +55,7 @@ * in order to write in /sys/class/backlight/driver/brightness file. *******************************************************************************/ +#include #include #include #include @@ -67,6 +68,7 @@ #define True 1 #define False 0 +static FILE* open_driver_file(const char *path, const char *driver, const char *mode); static int read_backlight(char *driver); static int read_max_backlight(char *driver); static int read_bl_power(char *driver); @@ -114,13 +116,10 @@ int lxqt_backlight_is_backlight_off() return bl_power; } -static int read_int(char *path) +static int read_int(const char *tpl, const char *driver) { - FILE *in = fopen(path, "r"); + FILE *in = open_driver_file(tpl, driver, "r"); if( in == NULL ) { - char buffer[1024]; - sprintf(buffer, "Couldn't open %s", path); - perror(buffer); return -1; } int value; @@ -132,25 +131,40 @@ static int read_int(char *path) return value; } +static FILE* open_driver_file(const char *tpl, const char *driver, const char *mode) +{ + char path[PATH_MAX]; + int res; + + res = snprintf(path, PATH_MAX, tpl, driver); + + if( res <= 0 || res >= PATH_MAX ) { + path[0] = '\0'; + return NULL; + } + + FILE *ret = fopen(path, mode); + + if( ret == NULL ) { + fprintf(stderr, "Couldn't open %s: %s\n", path, strerror(errno)); + } + + return ret; +} + static int read_backlight(char *driver) { - char path[1024]; - sprintf(path, "/sys/class/backlight/%s/actual_brightness", driver); - return read_int(path); + return read_int("/sys/class/backlight/%s/actual_brightness", driver); } static int read_max_backlight(char *driver) { - char path[1024]; - sprintf(path, "/sys/class/backlight/%s/max_brightness", driver); - return read_int(path); + return read_int("/sys/class/backlight/%s/max_brightness", driver); } static int read_bl_power(char *driver) { - char path[1024]; - sprintf(path, "/sys/class/backlight/%s/bl_power", driver); - return read_int(path); + return read_int("/sys/class/backlight/%s/bl_power", driver); } typedef enum {FIRMWARE, PLATFORM, RAW, OTHER, N_BACKLIGHT} BackligthTypes; @@ -163,7 +177,7 @@ char *lxqt_backlight_backend_get_driver() char *drivers[N_BACKLIGHT]; char *driver; int n; - char path[1024], type[1024]; + char type[1024]; for(n=0;nd_name, ".") || !strcmp(dp->d_name, "..") ) continue; driver = dp->d_name; - sprintf(path, "/sys/class/backlight/%s/type", driver); - FILE *in = fopen(path, "r"); + FILE *in = open_driver_file("/sys/class/backlight/%s/type", driver, "r"); if( in == NULL ) continue; int ok = fscanf(in, "%s", type); diff --git a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c index 9586485..45c9281 100644 --- a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c @@ -35,14 +35,10 @@ */ static void set_bl_power(char *driver, int value) { - char path[1024]; - sprintf(path, "/sys/class/backlight/%s/bl_power", driver); - FILE *out = fopen(path, "w"); + FILE *out = open_driver_file("/sys/class/backlight/%s/bl_power", driver, "w"); if( out != NULL ) { fprintf(out, "%d", value); fclose(out); - } else { - perror("Couldn't open /sys/class/backlight/driver/bl_power"); } } @@ -50,14 +46,10 @@ static void set_bl_power(char *driver, int value) static void set_backlight(char *driver, int value) { if(value>0) { - char path[1024]; - sprintf(path, "/sys/class/backlight/%s/brightness", driver); - FILE *out = fopen(path, "w"); + FILE *out = open_driver_file("/sys/class/backlight/%s/brightness", driver, "w"); if( out != NULL ) { fprintf(out, "%d", value); fclose(out); - } else { - perror("Couldn't open /sys/class/backlight/driver/brightness"); } if(read_bl_power(driver) > 0) set_bl_power(driver, 0); @@ -247,4 +239,4 @@ int main(int argc, char *argv[]) change_blacklight(value, value_percent_ok); return 0; -} \ No newline at end of file +} From 318f85b2aca43ed6c9b4f6099f3d476bb8b9e1c2 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 16:36:41 +0200 Subject: [PATCH 2/8] lxqtbacklight: apply maximum string length to fscanf to prevent overflow --- lxqtbacklight/linux_backend/driver/libbacklight_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index 4d90c20..c7ce7ab 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -196,7 +196,8 @@ char *lxqt_backlight_backend_get_driver() FILE *in = open_driver_file("/sys/class/backlight/%s/type", driver, "r"); if( in == NULL ) continue; - int ok = fscanf(in, "%s", type); + // the maximum field width does not include '\0'! + int ok = fscanf(in, "%1023s", type); fclose(in); if( ok != EOF ) { // firmware control should be preferred to platform control should be preferred to raw control. From 628dbd33fcaf93b79334572782991ac8d4362f14 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 16:49:07 +0200 Subject: [PATCH 3/8] lxqtbacklight: removed useless commented out code --- .../linux_backend/driver/lxqtbacklight_backend.c | 34 ---------------------- 1 file changed, 34 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c index 45c9281..3dc7d7e 100644 --- a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c @@ -58,40 +58,6 @@ static void set_backlight(char *driver, int value) } } - -// static int read_int(char *path) -// { -// FILE *in = fopen(path, "r"); -// if( in == NULL ) { -// char buffer[1024]; -// sprintf(buffer, "Couldn't open %s", path); -// perror(buffer); -// return -1; -// } -// int value; -// int ok = fscanf(in, "%d", &value); -// fclose(in); -// if( ok == EOF ) { -// value = 0; -// } -// return value; -// } - - -// static int read_backlight(char *driver) -// { -// char path[1024]; -// sprintf(path, "/sys/class/backlight/%s/actual_brightness", driver); -// return read_int(path); -// } - -// static int read_max_backlight(char *driver) -// { -// char path[1024]; -// sprintf(path, "/sys/class/backlight/%s/max_brightness", driver); -// return read_int(path); -// } - static char *get_driver() { return lxqt_backlight_backend_get_driver(); From 8ddd5354c6e5175d0674ff77ff7093d9b4fdba3e Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 16:52:42 +0200 Subject: [PATCH 4/8] lxqtbacklight: constified char *driver, where appropriate --- lxqtbacklight/linux_backend/driver/libbacklight_backend.c | 12 ++++++------ lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index c7ce7ab..bab2c48 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -69,9 +69,9 @@ #define False 0 static FILE* open_driver_file(const char *path, const char *driver, const char *mode); -static int read_backlight(char *driver); -static int read_max_backlight(char *driver); -static int read_bl_power(char *driver); +static int read_backlight(const char *driver); +static int read_max_backlight(const char *driver); +static int read_bl_power(const char *driver); int lxqt_backlight_backend_get() { @@ -152,17 +152,17 @@ static FILE* open_driver_file(const char *tpl, const char *driver, const char *m return ret; } -static int read_backlight(char *driver) +static int read_backlight(const char *driver) { return read_int("/sys/class/backlight/%s/actual_brightness", driver); } -static int read_max_backlight(char *driver) +static int read_max_backlight(const char *driver) { return read_int("/sys/class/backlight/%s/max_brightness", driver); } -static int read_bl_power(char *driver) +static int read_bl_power(const char *driver) { return read_int("/sys/class/backlight/%s/bl_power", driver); } diff --git a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c index 3dc7d7e..4c895ae 100644 --- a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c @@ -33,7 +33,7 @@ * @param driver is the driver to use * @param value 0 turns on backlight, 4 turns off backlight */ -static void set_bl_power(char *driver, int value) +static void set_bl_power(const char *driver, int value) { FILE *out = open_driver_file("/sys/class/backlight/%s/bl_power", driver, "w"); if( out != NULL ) { @@ -43,7 +43,7 @@ static void set_bl_power(char *driver, int value) } -static void set_backlight(char *driver, int value) +static void set_backlight(const char *driver, int value) { if(value>0) { FILE *out = open_driver_file("/sys/class/backlight/%s/brightness", driver, "w"); From 25114361c2e3bd60f917dde86807b4f1e72e7dc3 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 17:11:08 +0200 Subject: [PATCH 5/8] lxqtbacklight: centralized error output on empty backlight dir - added missing newline to output - also catch error in --stdin case, this could lead to a SIGSEGV on some libc implementations and results in trying to open /sys/class/backlight/(null)/max_brightness with glibc. --- .../linux_backend/driver/libbacklight_backend.c | 18 +++++++++++------- .../linux_backend/driver/lxqtbacklight_backend.c | 7 +++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index bab2c48..484e3d5 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -72,12 +72,12 @@ static FILE* open_driver_file(const char *path, const char *driver, const char * static int read_backlight(const char *driver); static int read_max_backlight(const char *driver); static int read_bl_power(const char *driver); +static const char *sysfs_backlight_dir = "/sys/class/backlight"; int lxqt_backlight_backend_get() { char *driver = lxqt_backlight_backend_get_driver(); if( driver == NULL ) { - fprintf(stderr, "Error: /sys/class/backlight is empty."); return -1; } int value = read_backlight(driver); @@ -89,7 +89,6 @@ int lxqt_backlight_backend_get_max() { char *driver = lxqt_backlight_backend_get_driver(); if( driver == NULL ) { - fprintf(stderr, "Error: /sys/class/backlight is empty."); return -1; } int value = read_max_backlight(driver); @@ -108,7 +107,6 @@ int lxqt_backlight_is_backlight_off() { char *driver = lxqt_backlight_backend_get_driver(); if( driver == NULL ) { - fprintf(stderr, "Error: /sys/class/backlight is empty."); return -1; } int bl_power = read_bl_power(driver); @@ -182,8 +180,8 @@ char *lxqt_backlight_backend_get_driver() for(n=0;n 0 && value <= max_value) { From 3b1610dd32e015b2625008235be44f51cae4a8c8 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 13 Jun 2018 17:23:49 +0200 Subject: [PATCH 6/8] lxqtbacklight: removed extra whitespace --- lxqtbacklight/linux_backend/driver/libbacklight_backend.c | 6 +++--- lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index 484e3d5..733fe9e 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -171,12 +171,12 @@ char *lxqt_backlight_backend_get_driver() { DIR *dirp; struct dirent *dp; - + char *drivers[N_BACKLIGHT]; char *driver; int n; char type[1024]; - + for(n=0;n Date: Wed, 13 Jun 2018 18:01:36 +0200 Subject: [PATCH 7/8] lxqtbacklight: fix and improve command line parameter handling - the percent variant didn't work before, because the logic was wrong - detect non-numerical unsupported arguments and print usage in this case - support 0% and 100% but avoid turning off the backlight completely --- .../linux_backend/driver/lxqtbacklight_backend.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c index 3e545a8..836e3f3 100644 --- a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "libbacklight_backend.h" #include "libbacklight_backend.c" @@ -83,9 +84,14 @@ static void change_blacklight(int value, int percent_ok) return; } int max_value = read_max_backlight(driver); - if(percent_ok) + if(percent_ok) { value = (float)(max_value*value)/100.0; - if(value0) { + if( value == 0 ) { + // avoid switching off backlight but support zero as lowest value + value = 1; + } + } + if(value<=max_value && value>0) { set_backlight(driver, value); } free(driver); @@ -186,10 +192,10 @@ int main(int argc, char *argv[]) } if( !strcmp(argv[n], "--stdin") ) { set_backlight_from_stdin(); return 0; - } else if ( argv[n][0] != '-' ) { - value = atoi(argv[1]); - } else if ( argv[n][0] != '%' && strlen(argv[n])==1 ) { + } else if ( !strcmp(argv[n], "%") ) { value_percent_ok = True; + } else if ( isdigit(argv[n][0]) ) { + value = atoi(argv[n]); } else { help(argv[0]); return 0; From 7bd68881d6a65541437f2e4dfad0f6749b12034b Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Thu, 14 Jun 2018 11:28:14 +0200 Subject: [PATCH 8/8] lxqtbacklight: only pass basename into open_driver_file() this way the sysfs directory path is centrally defined and the code becomes better readable. --- .../linux_backend/driver/libbacklight_backend.c | 18 +++++++++--------- .../linux_backend/driver/lxqtbacklight_backend.c | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c index 733fe9e..1282706 100644 --- a/lxqtbacklight/linux_backend/driver/libbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/libbacklight_backend.c @@ -68,7 +68,7 @@ #define True 1 #define False 0 -static FILE* open_driver_file(const char *path, const char *driver, const char *mode); +static FILE* open_driver_file(const char *file, const char *driver, const char *mode); static int read_backlight(const char *driver); static int read_max_backlight(const char *driver); static int read_bl_power(const char *driver); @@ -114,9 +114,9 @@ int lxqt_backlight_is_backlight_off() return bl_power; } -static int read_int(const char *tpl, const char *driver) +static int read_int(const char *file, const char *driver) { - FILE *in = open_driver_file(tpl, driver, "r"); + FILE *in = open_driver_file(file, driver, "r"); if( in == NULL ) { return -1; } @@ -129,12 +129,12 @@ static int read_int(const char *tpl, const char *driver) return value; } -static FILE* open_driver_file(const char *tpl, const char *driver, const char *mode) +static FILE* open_driver_file(const char *file, const char *driver, const char *mode) { char path[PATH_MAX]; int res; - res = snprintf(path, PATH_MAX, tpl, driver); + res = snprintf(path, PATH_MAX, "%s/%s/%s", sysfs_backlight_dir, driver, file); if( res <= 0 || res >= PATH_MAX ) { path[0] = '\0'; @@ -152,17 +152,17 @@ static FILE* open_driver_file(const char *tpl, const char *driver, const char *m static int read_backlight(const char *driver) { - return read_int("/sys/class/backlight/%s/actual_brightness", driver); + return read_int("actual_brightness", driver); } static int read_max_backlight(const char *driver) { - return read_int("/sys/class/backlight/%s/max_brightness", driver); + return read_int("max_brightness", driver); } static int read_bl_power(const char *driver) { - return read_int("/sys/class/backlight/%s/bl_power", driver); + return read_int("bl_power", driver); } typedef enum {FIRMWARE, PLATFORM, RAW, OTHER, N_BACKLIGHT} BackligthTypes; @@ -191,7 +191,7 @@ char *lxqt_backlight_backend_get_driver() if( !strcmp(dp->d_name, ".") || !strcmp(dp->d_name, "..") ) continue; driver = dp->d_name; - FILE *in = open_driver_file("/sys/class/backlight/%s/type", driver, "r"); + FILE *in = open_driver_file("type", driver, "r"); if( in == NULL ) continue; // the maximum field width does not include '\0'! diff --git a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c index 836e3f3..8886a74 100644 --- a/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c +++ b/lxqtbacklight/linux_backend/driver/lxqtbacklight_backend.c @@ -36,7 +36,7 @@ */ static void set_bl_power(const char *driver, int value) { - FILE *out = open_driver_file("/sys/class/backlight/%s/bl_power", driver, "w"); + FILE *out = open_driver_file("bl_power", driver, "w"); if( out != NULL ) { fprintf(out, "%d", value); fclose(out); @@ -47,7 +47,7 @@ static void set_bl_power(const char *driver, int value) static void set_backlight(const char *driver, int value) { if(value>0) { - FILE *out = open_driver_file("/sys/class/backlight/%s/brightness", driver, "w"); + FILE *out = open_driver_file("brightness", driver, "w"); if( out != NULL ) { fprintf(out, "%d", value); fclose(out);