some cleanup/bug fixes

Makefile

@@ -1,19 +1,27 @@
+.PHONY:all
 all: obj/dep rexbacklight rexledctl
+	strip --strip-unneeded rexbacklight
+	strip --strip-unneeded rexledctl
+
+debug: obj/dep rexbacklight rexledctl
+
+CFLAGS:=-Wall -Wextra -pedantic -std=c11
+debug:CFLAGS+=-g
 
 rexbacklight: obj/rexbacklight.o obj/common.o obj/cmd.o
-	gcc -o rexbacklight $^
+	gcc -o $@ $^
 
 rexledctl: obj/rexledctl.o obj/common.o obj/cmd.o
-	gcc -o rexledctl $^
+	gcc -o $@ $^
 
 obj/rexledctl.o: src/rexbacklight.c
-	gcc -c -g -o $@ $< -DREXLEDCTL -I"include"  -MMD -MP -MF"obj/dep/rexledctl.d"
+	gcc -c $(CFLAGS) -o $@ $< -DREXLEDCTL -I"include"  -MMD -MP -MF"obj/dep/rexledctl.d"
 
 obj/%.o: src/%.c
-	gcc -c -g -o $@ $< -DREXBACKLIGHT -I"include"  -MMD -MP -MF"obj/dep/$*.d"
+	gcc -c $(CFLAGS) -o $@ $< -DREXBACKLIGHT -I"include"  -MMD -MP -MF"obj/dep/$*.d"
 
 .PHONY: install
-install: rexbacklight rexledctl
+install: all
 	install -m755 -o root -g root -s rexbacklight /usr/bin/rexbacklight
 	install -m755 -o root -g root -s rexledctl /usr/bin/rexledctl
 	install -m600 -o root -g root rules/91-backlight.rules /etc/udev/rules.d/91-backlight.rules
@@ -22,11 +30,11 @@ install: rexbacklight rexledctl
 
 .PHONY: uninstall
 uninstall:
-	rm /usr/bin/rexbacklight
+	rm -f /usr/bin/rexbacklight
-	rm /usr/bin/rexledctl
+	rm -f /usr/bin/rexledctl
-	rm /etc/udev/rules.d/91-backlight.rules
+	rm -f /etc/udev/rules.d/91-backlight.rules
-	rm /etc/udev/rules.d/91-leds.rules
+	rm -f /etc/udev/rules.d/91-leds.rules
-	rm /etc/init.d/rexbacklight
+	rm -f /etc/init.d/rexbacklight
 
 obj:
 	mkdir -p $@

TODO

@@ -4,5 +4,6 @@
 xbacklight compat option (-get/-set/-inc/-dec/-help/-time/-steps)
 #allow fade in and fade out
 a lot of cleanup
-rename "backlight" things to "device" things because led/backlight ambiguity
+#rename "backlight" things to "device" things because led/backlight ambiguity
-better format output of --get
+#better format output of --get
+dynamic formatting for --get

include/cmd.h

@@ -25,6 +25,7 @@
 #define OP_LIST 8
 #define OP_GET 128
 #define OP_NONE 0
+#define OP_USAGE 255
 
 struct cmd_arg{
 	const char* lopt;

include/common.h

@@ -25,10 +25,13 @@
 #define RETVAL_INVALID_DIR          2
 #define RETVAL_UNRECOGNIZED_OPTION  3
 #define RETVAL_MISSING_OPTION       4
-#define RETVAL_INVALID_DEVICE      -5
+#define RETVAL_INVALID_DEVICE       5
 #define RETVAL_MEM_ERROR            6
+#define RETVAL_INTERNAL_ERROR       7
 #define RETVAL_SUCCESS              EXIT_SUCCESS
+extern int return_value;
 
+void mem_error(void);
 _Noreturn void usage(int exit_val);
 
 

src/cmd.c

@@ -62,14 +62,16 @@ void free_cmd_args(struct arg_values* a){
 		if(i == argc - 1){ \
 			fprintf(stderr, "Missing argument to '%s'\n\n", argv[i]); \
 			free_cmd_args(&ret); \
-			usage(RETVAL_MISSING_OPTION); \
+			return_value = RETVAL_MISSING_OPTION; \
+			return (struct arg_values){.operation = OP_USAGE}; \
 		} \
 	}while(0)
 #define UNRECOGNIZED_OPTION() \
 	do{ \
 		fprintf(stderr, "Unrecognized command line option '%s'\n\n", argv[i]); \
 		free_cmd_args(&ret); \
-		usage(RETVAL_UNRECOGNIZED_OPTION); \
+		return_value = RETVAL_UNRECOGNIZED_OPTION; \
+		return (struct arg_values){.operation = OP_USAGE}; \
 	}while(0);
 
 struct arg_values init_arg_values(void){
@@ -80,9 +82,10 @@ struct arg_values init_arg_values(void){
 struct arg_values process_cmd_args(int argc, char** argv){
 	struct arg_values ret = init_arg_values();
 	struct arg_values* curr = &ret;
+	int i;
 
 	//Skip argv[0]
-	for(int i = 1;i < argc;i++){
+	for(i = 1;i < argc;++i){
 
 		//Check for switches
 		if(CHECK_OPTION(GET, argv[i])){
@@ -104,6 +107,12 @@ struct arg_values process_cmd_args(int argc, char** argv){
 		else if(CHECK_OPTION(DEVICE, argv[i])){
 			CHECK_NEXT_ARG();
 			curr->next = malloc(sizeof(struct arg_values));
+			if(!curr->next){
+				mem_error();
+				free_cmd_args(&ret);
+				return_value = RETVAL_MEM_ERROR;
+				return (struct arg_values){.operation = OP_USAGE};
+			}
 			*(curr->next) = init_arg_values();
 			curr = curr->next;
 			curr->device = argv[++i];
@@ -117,7 +126,7 @@ struct arg_values process_cmd_args(int argc, char** argv){
 		}
 		else if(CHECK_OPTION(HELP, argv[i])){
 			free_cmd_args(&ret);
-			usage(RETVAL_SUCCESS);
+			return (struct arg_values){.operation = OP_USAGE};
 		}
 
 		else if(!strcmp(argv[i], "max")){
@@ -154,7 +163,8 @@ struct arg_values process_cmd_args(int argc, char** argv){
 		}
 
 		else{
-			for(int j = 0; j < 3;j++){
+			int j;
+			for(j = 0; j < 3;++j){
 				if(argv[i][j] == '\0')
 					break;
 				if(argv[i][j] < '0' || argv[i][j] > '9')
@@ -169,14 +179,16 @@ struct arg_values process_cmd_args(int argc, char** argv){
 	if(!ret.operation){
 		if(!ret.next){
 			fprintf(stderr, "No operation requested!\n\n");
-			usage(RETVAL_MISSING_OPTION);
+			return_value = RETVAL_MISSING_OPTION;
+			return (struct arg_values){.operation = OP_USAGE};
 		}
 		//if there is a device specified, make sure each one has a requested operation
 		for(curr = ret.next;curr;curr = curr->next){
 			if(!curr->operation){
 				fprintf(stderr, "No operation requested for device '%s'!\n\n", curr->device);
 				free_cmd_args(&ret);
-				usage(RETVAL_MISSING_OPTION);
+				return_value = RETVAL_MISSING_OPTION;
+				return (struct arg_values){.operation = OP_USAGE};
 			}
 		}
 	//If there is a globally defined operation, apply it to the devices with no operation request
@@ -221,8 +233,9 @@ int process_op(struct arg_values* arg, float min, float current, float max){
 			else
 				return delta;
 		default:
-			fprintf(stderr, "Internal error, please fix for me ;)\n");
+			fprintf(stderr, "Unrecognized operation requested!\n");
-			exit(-99);
+			fprintf(stderr, "Internal error, plea1se fix for me ;)\n");
+			return_value = RETVAL_INTERNAL_ERROR;
 	}
 	return current;
 }

src/common.c

@@ -21,14 +21,21 @@
 
 #include <stdio.h>
 
+int return_value = RETVAL_SUCCESS;
+
+void mem_error(void){
+	fprintf(stderr, "Failed to allocate memory! Unable to continue!\n");
+}
+
 //name of the program being run so that we print the correct name in the usage
 extern const char* executable_name;
 
 _Noreturn void usage(int exit_val){
+	int i;
 	printf("Usage: %s [argument] [options] [argument]\n\n", executable_name);
 
 	printf("Options:\n");
-	for(int i = 0;i < rexbacklight_args_length;i++){
+	for(i = 0;i < rexbacklight_args_length;++i){
 		printf("  %s|%s\n", rexbacklight_args[i].lopt, rexbacklight_args[i].sopt);
 		printf("    %s\n", rexbacklight_args[i].desc);
 	}

src/rexbacklight.c

@@ -31,20 +31,19 @@
 
 //name of the program being run so that we print the correct name in the usage
 
-//This is where backlight devices can be found in sysfs
 #ifdef REXBACKLIGHT
-static const char* backlight_dir = "/sys/class/backlight/";
+//This is where backlight devices can be found in sysfs
+static const char* device_dir = "/sys/class/backlight/";
 const char* executable_name = "rexbacklight";
 #elif defined(REXLEDCTL)
-static const char* backlight_dir = "/sys/class/leds";
+//This is where led devices can be found in sysfs
+static const char* device_dir = "/sys/class/leds";
 const char* executable_name = "rexledctl";
 #else
 #error "UNDEFINED PROGRAM NAME!"
 #endif
-static const char* backlight_file = "brightness";
+static const char* brightness_file = "brightness";
-static const char* max_backlight_file = "max_brightness";
+static const char* max_brightness_file = "max_brightness";
-
-static int return_value = RETVAL_SUCCESS;
 
 //Used to store a dynamic array of dynamic strings and number of strings
 struct string_array{
@@ -64,61 +63,77 @@ void io_error(const char* error, const char* type, const char* name){
 void io_error_2(const char* error, const char* type, const char* name, const char* name2){
 	fprintf(stderr, "Unable to %s %s '%s/%s'!\n", error, type, name, name2);
 }
-void mem_error(void){
+void io_error_3(const char* error, const char* type, const char* name, const char* name2, const char* name3){
-	fprintf(stderr, "Failed to allocate memory! Unable to continue!\n");
+	fprintf(stderr, "Unable to %s %s '%s/%s/%s'!\n", error, type, name, name2, name3);
 }
 
 //Clean up a string array
 void free_string_array(struct string_array* s){
-	for(int i = 0;i < s->size;i++)
+	int i;
+	for(i = 0;i < s->size;++i)
 		free(s->list[i]);
 	free(s->list);
 }
 
-//Get a list of backlight devices in sysfs
+//Get a list of led/backlight devices in sysfs
-struct string_array get_backlight_sources(void){
+struct string_array get_device_sources(void){
-
 	DIR* fd;
 	struct dirent* dir;
+	struct string_array arr = {0};
+	int list_size = 8;
 
-	fd = opendir(backlight_dir);
+	fd = opendir(device_dir);
-	//If the dir can't be opened, return no backlight devices
 	if(!fd){
-		io_error(IO_ERROR_OPEN, IO_ERROR_DIR, backlight_dir);
+		io_error(IO_ERROR_OPEN, IO_ERROR_DIR, device_dir);
 		return_value = RETVAL_INVALID_DIR;
-		return (struct string_array){.list = NULL, .size = 0};
+		return arr;
 	}
 
-	int i, list_size = 8;
+	arr.list = malloc(sizeof(const char*) * list_size);
-	char** list = malloc(sizeof(const char*) * list_size);
+	if(!arr.list){
+		mem_error();
+		return_value = RETVAL_MEM_ERROR;
+		return arr;
+	}
 
-	for(i = 0;(dir = readdir(fd));){
+#define MEMORY_ERROR() do{mem_error();return_value = RETVAL_MEM_ERROR;free_string_array(&arr);closedir(fd);}while(0)
 
-		//Don't include "." or ".." in directory list
+	for(arr.size = 0;(dir = readdir(fd));){
-		if(!strcmp(dir->d_name, ".") || !strcmp(dir->d_name, "..")){
+		int slen;
+		if(!strncmp(dir->d_name, ".", 1) || !strncmp(dir->d_name, "..", 2)){
 			continue;
 		}
 
-		//Resize list if more than 8 backlight devices exist (not likely)
+		//Resize list if more than 8 led/backlight devices exist (not likely)
-		if(i >= list_size){
+		if(arr.size >= list_size){
+			int j;
 			char** tmp = malloc(sizeof(const char*) * (list_size += 8));
-			for(int j = 0;j < i;j++){
+			if(!tmp){
-				tmp[j] = list[j];
+				MEMORY_ERROR();
+				return (struct string_array){0};
 			}
-			free(list);
+			for(j = 0;j < arr.size;++j){
-			list = tmp;
+				tmp[j] = arr.list[j];
+			}
+			free(arr.list);
+			arr.list = tmp;
 		}
 
-		int slen = strlen(dir->d_name);
+		slen = strlen(dir->d_name);
-		list[i] = malloc(slen + 1);
+		arr.list[arr.size] = malloc(slen + 1);
-		strcpy(list[i], dir->d_name);
+		if(!arr.list[arr.size]){
-		list[i][slen] = 0;
+			MEMORY_ERROR();
-		i++;
+			return (struct string_array){0};
 		}
+		strcpy(arr.list[arr.size], dir->d_name);
+		arr.list[arr.size][slen] = 0;
+		++arr.size;
+	}
+
+#undef MEMORY_ERROR
 
 	closedir(fd);
-
+	return arr;
-	return (struct string_array){.list = list, .size = i};
 }
 
 
@@ -142,7 +157,7 @@ float get_brightness(const char* file){
 }
 
 //update brightness incrementally over requested millisecond time interval
-void fade_out(int iv, int fv, int ms_duration, int steps){
+void fade_out(const char* device_name, int iv, int fv, int ms_duration, int steps){
 	FILE* fd;
 	struct timeval start, end;
 	double tdelta = 0;        //amount of time that has passed
@@ -154,9 +169,9 @@ void fade_out(int iv, int fv, int ms_duration, int steps){
 	while(ms_duration > tdelta){
 		//write
 		gettimeofday(&start, NULL);
-		fd = fopen(backlight_file, "w+");
+		fd = fopen(brightness_file, "w+");
 		if(!fd){
-			io_error_2(IO_ERROR_OPEN, IO_ERROR_FILE, backlight_dir, backlight_file);
+			io_error_3(IO_ERROR_OPEN, IO_ERROR_FILE, device_dir, device_name, brightness_file);
 			return_value = RETVAL_INVALID_FILE;
 			return;
 		}
@@ -178,9 +193,9 @@ void fade_out(int iv, int fv, int ms_duration, int steps){
 		//calc end of next step
 		step_delta += step_inc;
 	}
-	fd = fopen(backlight_file, "w+");
+	fd = fopen(brightness_file, "w+");
 	if(!fd){
-		io_error_2(IO_ERROR_OPEN, IO_ERROR_FILE, backlight_dir, backlight_file);
+		io_error_3(IO_ERROR_OPEN, IO_ERROR_FILE, device_dir, device_name, brightness_file);
 		return_value = RETVAL_INVALID_FILE;
 		return;
 	}
@@ -189,29 +204,38 @@ void fade_out(int iv, int fv, int ms_duration, int steps){
 	fclose(fd);
 }
 
-//Write value to backlight files
+//Write value to device files
-void do_assignment(struct arg_values* arg){
+void do_assignment(struct arg_values* arg, const char* device){
-	int start = get_brightness(backlight_file);
+	int start = get_brightness(brightness_file);
-	int out = process_op(arg, 0, start, get_brightness(max_backlight_file));
+	int out = process_op(arg, 0, start, get_brightness(max_brightness_file));
-	fade_out(start, out, arg->fade_duration, arg->fade_steps);
+	fade_out(device, start, out, arg->fade_duration, arg->fade_steps);
 }
 
 //Run get operation
 void do_get(const char* device){
-	fprintf(stdout, "%s: %.2f\n", device, get_brightness(backlight_file) / get_brightness(max_backlight_file) * 100);
+	float curr, max;
+	curr = get_brightness(brightness_file);
+	max = get_brightness(max_brightness_file);
+	if(return_value == RETVAL_INVALID_FILE && (!curr || !max)){
+		fprintf(stdout, "%s", device);
+	}else{
+		fprintf(stdout, "%-25s %.2f\n", device, get_brightness(brightness_file) / get_brightness(max_brightness_file) * 100);
+	}
 }
 
 //Run list operation
 void do_list(struct string_array* names){
-	for(int i = 0;i < names->size;i++)
+	int i;
+	for(i = 0;i < names->size;++i)
 		fprintf(stdout, "%s\n", names->list[i]);
 }
 
 //If devices were specified, this function will run
 void individual_device_loop(struct arg_values* a){
-	for(struct arg_values* curr = a->next;curr;curr = curr->next){
+	struct arg_values* curr;
+	for(curr = a->next;curr;curr = curr->next){
 		if(chdir(curr->device)){
-			io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, backlight_dir, curr->device);
+			io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, device_dir, curr->device);
 			return_value = RETVAL_INVALID_DIR;
 			continue;
 		}
@@ -219,43 +243,45 @@ void individual_device_loop(struct arg_values* a){
 			case OP_SET:
 			case OP_INC:
 			case OP_DEC:
-				do_assignment(curr);
+				do_assignment(curr, curr->device);
 				break;
 			case OP_GET:
 				do_get(curr->device);
 				break;
 		}
+		chdir(device_dir);
 	}
 }
 
 //If no devices were specified, this function will run
-void all_device_loop(struct string_array* backlight_names, struct arg_values* args){
+void all_device_loop(struct string_array* device_names, struct arg_values* args){
+	int i;
 	switch(args->operation){
 		case OP_SET:
 		case OP_INC:
 		case OP_DEC:
-			for(int i = 0;i < backlight_names->size;i++){
+			for(i = 0;i < device_names->size;++i){
-				if(chdir(backlight_names->list[i])){
+				if(chdir(device_names->list[i])){
-					io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, backlight_dir, backlight_names->list[i]);
+					io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, device_dir, device_names->list[i]);
 					return_value = RETVAL_INVALID_DIR;
 					continue;
 				}
-				do_assignment(args);
+				do_assignment(args, device_names->list[i]);
-				chdir(backlight_dir);
+				chdir(device_dir);
 			}
 			break;
 		case OP_LIST:
-			do_list(backlight_names);
+			do_list(device_names);
 			break;
 		case OP_GET:
-			for(int i = 0;i < backlight_names->size;i++){
+			for(i = 0;i < device_names->size;++i){
-				if(chdir(backlight_names->list[i])){
+				if(chdir(device_names->list[i])){
-					io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, backlight_dir, backlight_names->list[i]);
+					io_error_2(IO_ERROR_OPEN, IO_ERROR_DIR, device_dir, device_names->list[i]);
 					return_value = RETVAL_INVALID_DIR;
 					continue;
 				}
-				do_get(backlight_names->list[i]);
+				do_get(device_names->list[i]);
-				chdir(backlight_dir);
+				chdir(device_dir);
 			}
 			break;
 	}
@@ -265,18 +291,29 @@ void all_device_loop(struct string_array* backlight_names, struct arg_values* ar
 int main(int argc, char** argv){
 
 	struct arg_values args; //A linked list of devices and the requested settings.
-	struct string_array backlight_names; //List of all backlight devices in sysfs.
+	struct string_array device_names; //List of all led/backlight devices in sysfs.
-
+	struct arg_values* curr;
+	int starting_dir_max_len = pathconf(".", _PC_PATH_MAX);
+	char* starting_dir;
 	args = process_cmd_args(argc, argv);
-	backlight_names = get_backlight_sources();
+	if(args.operation == OP_USAGE){
+		usage(return_value);
+	}
+
+	starting_dir = malloc(starting_dir_max_len);
+	if(!starting_dir){
+		mem_error();
+		return RETVAL_MEM_ERROR;
+	}
+	device_names = get_device_sources();
 
 //Macro for easy memory cleaning
-#define CLEANUP() do{free_string_array(&backlight_names);free_cmd_args(&args);}while(0)
+#define CLEANUP() do{free(starting_dir);free_string_array(&device_names);free_cmd_args(&args);}while(0)
 
 
 
-	//If there are no backlights, we can't do anything
+	//If there are no led/backlights, we can't do anything
-	if(backlight_names.size == 0){
+	if(device_names.size == 0){
 #ifdef REXBACKLIGHT
 		fprintf(stderr, "No backlights devices found!\n");
 #elif defined(REXLEDCTL)
@@ -289,9 +326,10 @@ int main(int argc, char** argv){
 	}
 
 	//Make sure all explicit devices actually exist
-	for(struct arg_values* curr = args.next;curr;curr = curr->next){
+	for(curr = args.next;curr;curr = curr->next){
-		for(int i = 0;i < backlight_names.size;i++){
+		int i;
-			if(!strcmp(curr->device, backlight_names.list[i])){
+		for(i = 0;i < device_names.size;++i){
+			if(!strcmp(curr->device, device_names.list[i])){
 				goto continue_outer;
 			}
 		}
@@ -301,24 +339,13 @@ int main(int argc, char** argv){
 		continue_outer:;
 	}
 
-	int starting_dir_max_len = pathconf(".", _PC_PATH_MAX);
-	char* starting_dir = malloc(starting_dir_max_len);
-	if(!starting_dir){
-		mem_error();
-		CLEANUP();
-		return RETVAL_MEM_ERROR;
-	}
 	if(!getcwd(starting_dir, starting_dir_max_len)){
 		fprintf(stderr, "Unable to determine starting dir!\nWill be unable to return to it on completion!\n");
 	}
 
-#undef CLEANUP
+	//Change to the base directory for all sysfs leds/backlights
-#define CLEANUP() do{free_string_array(&backlight_names);free_cmd_args(&args);free(starting_dir);}while(0)
+	if(chdir(device_dir)){
-
+		io_error(IO_ERROR_READ, IO_ERROR_DIR, device_dir);
-
-	//Change to the base directory for all sysfs backlights
-	if(chdir(backlight_dir)){
-		io_error(IO_ERROR_READ, IO_ERROR_DIR, backlight_dir);
 		CLEANUP();
 		return RETVAL_INVALID_DIR;
 	}
@@ -327,7 +354,7 @@ int main(int argc, char** argv){
 	if(args.next){
 		individual_device_loop(&args);
 	}else{
-		all_device_loop(&backlight_names, &args);
+		all_device_loop(&device_names, &args);
 	}
 
 	//Return to start directory