removed unnecesary asterisk

.gitignore

@@ -1,2 +1,3 @@
 bin/*
+*.tar.gz
 !*.gitkeep

Makefile

@@ -1,8 +1,12 @@
 CC = gcc
-CFLAGS = -Wall -Wpedantic -Wextra -O2 -march=native
+CFLAGS = -Wall -Wpedantic -Wextra -O2 -march=native\
+ -DTHREADS=$(shell grep processor /proc/cpuinfo | wc -l)\
+ -DPOLLING_TIME=$(shell read -p "type seconds between temp checks >" P;\
+  echo $$P)
 SRC = src
 BINDIR = bin
 BIN = homemade_speedstep
+RELEASE_TAG = $(shell git describe)
 
 all: $(BIN)
 
@@ -12,13 +16,17 @@ $(BIN):
 debug:
 	$(CC) $(CFLAGS) -g $(SRC)/$(BIN).c -o $(BINDIR)/$(BIN)
 
-install:
-	cp $(BINDIR)/$(BIN) /usr/bin/$(BIN)
+release: clean
+	tar czf homemade_speedstep-$(RELEASE_TAG).tar.gz $(shell ls -A | grep -v "*.tar.gz")
 
-install-openrc:
+install:
+	mv $(BINDIR)/$(BIN) /usr/bin/$(BIN)
+
+install-openrc: install
 	cp init.d/$(BIN) /etc/init.d/$(BIN)
 	cp conf.d/$(BIN) /etc/conf.d/$(BIN)
 	rc-update add homemade_speedstep
 
 clean:
 	$(RM) -r $(BINDIR)/*
+	$(RM) ./*.tar.gz

README.md

@@ -1,5 +1,5 @@
 # Homemade Speedstep
-homemade_speedstep is a program I made while learning the C programming language to replace an old python script I've been using to keep my old laptop from shutting down due to thermal overload.
+homemade_speedstep is a program I made while learning the C programming language to replace a python script I've been using to keep my old laptop from shutting down due to thermal overload.
 
 If you're familiar with Intel's speedstep technology, this program attempts to imitate its behaviour, although in a much more crude way.
 
@@ -11,9 +11,7 @@ Read the **LICENSE** file distributed alongside this source code for more inform
 ## Requirements
 The program requires running as root, as it needs permission to read and write from the `/sys` directory.
 
-It also requires support for the cpufreq 'userspace' governor built into the kernel.
-
-The 'userspace' governor must be the currently running one.
+It also requires cpufreq support to be built into the kernel.
 ## Building
 Run `make` to compile the program, the binary will be placed in the `bin` directory.
 
@@ -26,13 +24,23 @@ Run `make debug` to compile the program with debug symbols (only useful if you w
 Run `make clean` to remove the compiled binary (note: only from `bin`, to remove from `/usr/bin` run `rm /usr/bin/homemade_speedstep`).
 
 ## How it works
-The program checks the available frequencies that the CPU can run at and it will try to keep it at the maximum that your system can run at before reaching your upper temperature threshold.
+The program checks the available frequency steps supported by your CPU and uses them to set the maximum frequency your cpufreq governor may choose for it to run at.
 
-If the temperature stays within the upper and lower bound, it will keep the current frequency.
+Every n ammount of seconds, set by the user at build time, the program reads the temperature of the CPU and two thresholds, a minimum and maximum temperature you provide it with, to set this maximum frequency.
 
-Upon reaching the upper threshold, it will step down the frequency by one step as defined by the available frequencies, down to the minimum frequency it can run at.
+Any time the temperature hits the upper threshold, the maximum frequency will be lowered by one step, until the temperature is below the threshold or the minimum step is reached.
 
-Once the temperature goes back down and it reaches the minimum threshold, the program will step up the frequency until it reaches the maximum again.
+Likewise, any time the temperature reaches the minimum threshold, the maximum frequency will be raised a step, until the temperature is no longer below the threshold or the maximum step is reached.
+
+If the temperature remains between the thresholds, the maximum frequency will remain unchanged.
+
+This behaviour could lead to the CPU running at a lower frequency than necessary at times, like in a situation where the upper threshold was momentarily hit, but the temperature then reameindd between both thresholds from then onwwards.
+
+Another unfortunate situation where the maximum frequency could unnecesarily remain below necessary would be when the lower thershold is reached and the temperature goes back up, but remains within both thresholds from then onwards.
+
+This is overcome with a third check that keeps track of the last time the maximum frequency was changed, and if the temperature is below the average of both minimum and maximum threshold; if so, the maximum frequency will be raised by a step.
+
+**Important**: this is not a complex algorithm to choose the frequency your CPU is running at at any given moment, that is the job of the CPU governor and out of scope of this program
 ## Usage
 You must provide the temperature at which you wish to step down the maximum frequency as first argument, and the temperature at which you wish to step up the maximum frequency as second argument; like so:
 
@@ -43,6 +51,8 @@ You must provide the temperature at which you wish to step down the maximum freq
 ---
 **Recomendation**: add a service to your init script that runs this program at startup in the background so you don't need to keep a terminal emulator open running it.
 
+If used in this way, The program's error output will be found at `/var/log/homemade_speedstep.err`
+
 Using OpenRC this would be done by adding the following in a `/etc/init.d/homemade_speedstep` file:
 ```
 #!/sbin/openrc-run
@@ -52,6 +62,7 @@ command=/usr/bin/homemade_speedstep
 command_args="${HSMAXTEMP} ${HSMINTEMP}"
 command_background=true
 pidfile="/run/${RC_SVCNAME}.pid"
+error_log=/var/log/homemade_speedstep.err
 
 checkconfig() {
   if [ ${HSMAXTEMP} -lt 100000 ] && [ ${HSMINTEMP} -lt ${HSMAXTEMP} ] ; then
@@ -74,7 +85,7 @@ stop_pre() {
 ```
 and the following in a `/etc/conf.d/homemade_speedstep` file:
 ```
-# /etc/conf.d/homemade_speedstep: config gile for /etc/init.d/homemade_speedstep
+# /etc/conf.d/homemade_speedstep: config file for /etc/init.d/homemade_speedstep
 # Change these to your liking
 # Upper temperature threshold
 HSMAXTEMP=82000

conf.d/homemade_speedstep

@@ -1,4 +1,4 @@
-# /etc/conf.d/homemade_speedstep: config gile for /etc/init.d/homemade_speedstep
+# /etc/conf.d/homemade_speedstep: config file for /etc/init.d/homemade_speedstep
 
 # Change these to your liking
 # Upper temperature threshold

init.d/homemade_speedstep

@@ -5,6 +5,7 @@ command=/usr/bin/homemade_speedstep
 command_args="${HSMAXTEMP} ${HSMINTEMP}"
 command_background=true
 pidfile="/run/${RC_SVCNAME}.pid"
+error_log=/var/log/homemade_speedstep.err
 
 checkconfig() {
   if [ ${HSMAXTEMP} -lt 100000 ] && [ ${HSMINTEMP} -lt ${HSMAXTEMP} ] ; then

src/homemade_speedstep.c

@@ -1,112 +1,22 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-
-#define POLLING_TIME 1
-#define CURGOV "/sys/devices/system/cpu/cpufreq/policy0/scaling_governor"
-#define FREQS "/sys/devices/system/cpu/cpufreq/policy0/scaling_available_frequencies"
-#define MAXSPDPATH "/sys/devices/system/cpu/cpufreq/policy0/scaling_max_freq"
-#define CURTEMPPATH "/sys/devices/virtual/thermal/thermal_zone0/temp"
-
-int comparestrings(char *a, char *b){
-  int c = 0;
-  while(a[c] != '\0' && b[c] != '\0'){
-    if(a[c] != b[c]){
-      return 1;
-    }
-    c++;
-  }
-  return 0;
-}
-
-int getfilec(FILE *file, char arr[]){
-  int c, i;
-  i = 0;
-  while((c = fgetc(file)) != EOF){
-    arr[i] = c;
-    i++;
-  }
-  arr[i] = '\0';
-  return i;
-}
-
-int powerof(int base, int exponent){
-  int i, result;
-  if (exponent == 0){
-    return result = 1;
-  }
-  result = base;
-  for(i = 0; i < exponent - 1; i++){
-    result *= base;
-  }
-  return result;
-}
-
-int stringtoint(char number[], int length){
-  int j, result;
-  result = 0;
-  j = 0;
-  while(length - 1 >= 0){
-    result += ((int)number[length - 1] - '0') * (powerof(10, j));
-    j++;
-    length--;
-  }
-  return result;
-}
-
-int stringlength(char string[]){
-  int i = 0;
-  while(string[i] != '\0'){
-    i++;
-  }
-  return i;
-}
-
-int splitstr(char** dest, char line[], int index1, int index2){
-  int offset = index1;
-  while (line[index1] != ' ' && index1 < stringlength(line)){
-    index1++;
-  }
-  char* p = malloc(sizeof (char)*index1);
-  int i;
-  for(i = 0; i < index1 - offset; i++){
-    p[i] = line[i + offset];
-  }
-  p[i] = '\0';
-  dest[index2] = p;
-  return index1 + 1;
-}
-
-int countblanks(char line[]){
-  int i = 0;
-  int j = 0;
-  while(line[i] != '\0'){
-    if (line[i] == ' '){
-      j++;
-    }
-    i++;
-  }
-  return j;
-}
-
-void newlinetonullbyte(char string[]){
-  int i = 0;
-  while (string[i] != '\0'){
-    if (string[i] == '\n'){
-      string[i] = '\0';
-      break;
-    }
-    i++;
-  }
-}
+#include "homemade_speedstep.h"
 
 int main(int argc, char** argv){
   /* check for no arguments or help */
   if (argc == 1 || comparestrings(argv[1], "-h") == 0
       || comparestrings(argv[1], "--help") == 0){
     fprintf(stderr, "usage: homemade_speedstep <max temp> <min temp>\n");
-    fprintf(stderr, "       homemade_speedstep [-h | --help]\n");
-    fprintf(stderr, "       -h | --help   prints out this help message\n");
+    fprintf(stderr, "       homemade_speedstep [-h | --help | -v | --version]\n");
+    fprintf(stderr, "       -h | --help      prints out this help message\n");
+    fprintf(stderr, "       -v | --version   prints out version number\n");
+    return 0;
+  }
+  /* print version number if asked to and exit */
+  if (comparestrings(argv[1], "-v") == 0
+      || comparestrings(argv[1], "--version") == 0){
+    fprintf(stderr, VERSION"\n");
     return 0;
   }
   /* check for correct ammount of arguments */
@@ -122,67 +32,44 @@ int main(int argc, char** argv){
      and that max is higher than min */
   if (maxtemp <= mintemp){
     fprintf(stderr, "error: maxtemp must be higher than mintemp\n");
+    fprintf(stderr, "       see homemade_speedstep --help\n");
     return 2;
   }
-  /* try to open scaling_current_governor, exit on fail */
-  FILE* govfile = fopen(CURGOV, "r");
-  if (govfile == NULL){
-    fprintf(stderr, "error: couldn't open scaling_current_governor\n");
-    fprintf(stderr, "       check that you have support for cpufreq\n");
-    fprintf(stderr, "       and the userspace governor in your kernel\n");
-    fprintf(stderr, "       also check that you're running this program\n");
-    fprintf(stderr, "       as root\n");
-    return 3;
-  }
-  /* assign content of govfile to a temporary char array */
-  char* gov = malloc(sizeof(char) * 20);
-  getfilec(govfile, gov);
-  /* check content of scaling_current_governor and exit on failure */
-  if (comparestrings(gov, "userspace\n") != 0){
-    fprintf(stderr, "error: userspace governor not set\n");
-    /* free memory and close file */
-    free(gov);
-    fclose(govfile);
-    return 4;
-  }
-  /* free memory and close file */
-  free(gov);
-  fclose(govfile);
   /* try to open scaling_available_frequencies, exit on fail */
-  FILE *freqsfile = fopen(FREQS, "r");
-  if (freqsfile == NULL){
-    fprintf(stderr, "error: couldn't open scaling_available_frequencies\n");
-    fprintf(stderr, "       check that you have support for cpufreq\n");
-    fprintf(stderr, "       in your kernel and that the program is\n");
-    fprintf(stderr, "       running as root\n");
+  FILE *freqsfile;
+  if ((freqsfile = fopen(FREQS, "r")) == NULL){
+    FILEERROR("read", "scaling_available_frequencies");
     return 5;
   }
   /* assign content of freqsfile to a temporary char array */
   char* freqsfilec = malloc(sizeof(char) * 200);
   getfilec(freqsfile, freqsfilec);
+  /* close file since we no loger use it */
+  fclose(freqsfile);
+  /* count the number of steps (should probably figure out a better way of doing this */
   int stepc = countblanks(freqsfilec);
   /* create a char ** array with pointers to freq steps */
   char ** steps = malloc(sizeof(char*) * stepc);
-  int index = 0;
+  int* index = malloc(sizeof(int*));
+  *index = 0;
   int i;
   for(i = 0; i < stepc; i++){
-    index = splitstr(steps, freqsfilec, index, i);
+    *index = splitstr(steps, freqsfilec, *index, i);
   }
-  /* free memory and close file */
+  /* free memory */
+  free(index);
   free(freqsfilec);
-  fclose(freqsfile);
   /* try to open scaling_maximum_frequency, exit on fail */
-  FILE* curmaxfile = fopen(MAXSPDPATH, "r");
-  if (curmaxfile == NULL){
-    fprintf(stderr, "error: couldn't open scaling_maximum_frequency\n");
-    fprintf(stderr, "       check that you have support for cpufreq\n");
-    fprintf(stderr, "       in your kernel and that the program is\n");
-    fprintf(stderr, "       running as root\n");
+  FILE* curmaxfile;
+  if ((curmaxfile = fopen(MAXSPDPREFIX"0"MAXSPDSUFFIX, "r")) == NULL){
+    FILEERROR("read", "scaling_maximum_frequency");
     return 6;
   }
   /* assign content of curmaxfile to a temporary char array */
   char *curmaxfreq = malloc(sizeof(char) * 20);
   getfilec(curmaxfile, curmaxfreq);
+  /* close file since we're done using it */
+  fclose(curmaxfile);
   newlinetonullbyte(curmaxfreq);
   /* figure out what step we're on */
   int curstep = -1;
@@ -192,9 +79,8 @@ int main(int argc, char** argv){
       break;
     }
   }
-  /* free memory and close file */
+  /* free memory */
   free(curmaxfreq);
-  fclose(curmaxfile);
   /* check if we didn't find our current step, exit on fail */
   if (curstep == -1){
     fprintf(stderr, "error: couldn't figure out the current frequency\n");
@@ -203,48 +89,58 @@ int main(int argc, char** argv){
     fprintf(stderr, "       running as root\n");
     return 7;
   }
+  /* these two go on the stack because we're going to need it throughout
+     the program's lifetime and also because they're small */
+  int timepassed = 0;
+  int temperature;
   FILE* curtempfile;
-  /* from here onwards an infinite while loop should start */
+  /* setup is ready, time to run the main loop */
   while((curtempfile = fopen(CURTEMPPATH, "r")) != NULL){
-    char* temperature = malloc(sizeof(char) * 8);
-    getfilec(curtempfile, temperature);
-    newlinetonullbyte(temperature);
-    /* check if current temp is higher than max and step down */
-    if (stringtoint(temperature, stringlength(temperature)) >= maxtemp
-        && curstep < stepc - 1){
-      curstep++;
-      curmaxfile = fopen(MAXSPDPATH, "w");
-      if (curmaxfile == NULL){
-        fprintf(stderr, "error: couldn't open scaling_maximum_frequency\n");
-        fprintf(stderr, "       check that you have support for cpufreq\n");
-        fprintf(stderr, "       in your kernel and that the program is\n");
-        fprintf(stderr, "       running as root\n");
-        return 6;
-      }
-      fprintf(curmaxfile, "%s\n", steps[curstep]);
-      fclose(curmaxfile);
+  /* we don't want timepassed to grow infinitely */
+    if (timepassed < 61){
+      timepassed++;
     }
-    /* check if current temp is lower than min and step down */
-    if (stringtoint(temperature, stringlength(temperature)) <= mintemp
-        && curstep > 0){
-      curstep--;
-      curmaxfile = fopen(MAXSPDPATH, "w");
-      if (curmaxfile == NULL){
-        fprintf(stderr, "error: couldn't open scaling_maximum_frequency\n");
-        fprintf(stderr, "       check that you have support for cpufreq\n");
-        fprintf(stderr, "       in your kernel and that the program is\n");
-        fprintf(stderr, "       running as root\n");
-        return 6;
-      }
-      fprintf(curmaxfile, "%s\n", steps[curstep]);
-      fclose(curmaxfile);
-    }
-    free(temperature);
+    /* 7 chars should be enough to store temp, but I put two more just in case
+       if temperature is above 9999999ºC there's bigger problems than memory */
+    char* char_temperature = malloc(sizeof(char) * 8);
+    getfilec(curtempfile, char_temperature);
+    /* close file since we're done using it */
     fclose(curtempfile);
+    newlinetonullbyte(char_temperature);
+    temperature = stringtoint(char_temperature, stringlength(char_temperature));
+    /* free memory */
+    free(char_temperature);
+    /* check if current temp is higher than max and step down */
+    if (temperature >= maxtemp && curstep < stepc - 1){
+      curstep++;
+      if (changefreq(THREADS, steps[curstep]) != 0){
+          FILEERROR("write to", "scaling_max_freq");
+        return 8;
+      }
+      timepassed = 0;
+    }
+    /* check if current temp is lower than min and step up */
+    if (temperature <= mintemp && curstep > 0){
+      curstep--;
+      if (changefreq(THREADS, steps[curstep]) != 0){
+          FILEERROR("write to", "scaling_max_freq");
+        return 8;
+      }
+      timepassed = 0;
+    }
+    /* check if a minute passed without step changes and temp is below minmax average */
+    if (timepassed > 60 && temperature <= (mintemp+maxtemp)/2 && curstep > 0){
+      curstep--;
+      if (changefreq(THREADS, steps[curstep]) != 0){
+          FILEERROR("write to", "scaling_max_freq");
+        return 8;
+      }
+      timepassed = 0;
+    }
     sleep(POLLING_TIME);
   }
-  /* cleanup for graceful exit */
+  /* something went wrong if we're here, make sure to free last dynamic memory */
   free(steps);
-  fclose(curtempfile);
-  return 0;
+  FILEERROR("read", "temp");
+  return 9;
 }

src/homemade_speedstep.h

@@ -0,0 +1,140 @@
+#ifndef STDIO_H
+#define STDIO_H
+#include <stdio.h>
+#endif
+#ifndef STDLIB_H
+#define STDLIB_H
+#include <stdlib.h>
+#endif
+#ifndef UNISTD_H
+#define UNISTD_H
+#include <unistd.h>
+#endif
+
+#ifndef HOMEMADE_SPEEDSTEP_H
+#define HOMEMADE_SPEEDSTEP_H
+
+#define VERSION "1.2"
+#define FREQS "/sys/devices/system/cpu/cpufreq/policy0/scaling_available_frequencies"
+#define MAXSPDPREFIX "/sys/devices/system/cpu/cpu"
+#define MAXSPDSUFFIX "/cpufreq/scaling_max_freq"
+#define CURTEMPPATH "/sys/devices/virtual/thermal/thermal_zone0/temp"
+#define FILEERROR(operation, filename) fprintf(stderr, "error: couldn't %s %s\n       check that you have support for cpufreq\n       in your kernel and that the program is\n       running as root\n", operation, filename)
+
+int changefreq(int threads, char* step); /* change max frequency of 'threads' to 'step', returns 0 on success */
+int comparestrings(char *a, char *b); /* no security checks, make sure at least one of the char arrays is null terminated, returns 0 on success */
+int getfilec(FILE *file, char arr[]); /* fills 'arr[]' with every character on '*file', returns the number of characters written to 'arr[]' as an int */
+int powerof(int base, int exponent); /* returns power of base to the exponent */
+int stringtoint(char number[], int length); /* returns an int of 'length' length from the string 'number', make sure string only has numbers in it or results will not be as expected */
+int stringlength(char string[]); /* no security checks, make sure string is null terminated */
+int splitstr(char** dest, char line[], int index1, int index2); /* add a substring from 'line' to 'dest'. index1 is offset of characters read from 'line' and index2 is the place in 'dest' to store the new char, returns int with new offset of characters read from 'line' + 1 */
+int countblanks(char line[]); /* return ammount of blanks in string */
+void newlinetonullbyte(char string[]); /* converts newline to nullbyte in a string, check that you only have newlines after the content you care about */
+
+int changefreq(int threads, char* step){
+  int i;
+  char filename[100];
+  FILE* file;
+  for (i=0; i < threads; i++){
+    sprintf(filename, MAXSPDPREFIX"%d"MAXSPDSUFFIX, i);
+    if ((file = fopen(filename, "w")) == NULL){
+      return 1;
+    }
+    fprintf(file, "%s\n", step);
+    fclose(file);
+  }
+  return 0;
+}
+
+int comparestrings(char *a, char *b){
+  int c = 0;
+  while(a[c] != '\0' && b[c] != '\0'){
+    if(a[c] != b[c]){
+      return 1;
+    }
+    c++;
+  }
+  return 0;
+}
+
+int getfilec(FILE *file, char arr[]){
+  int c, i;
+  i = 0;
+  while((c = fgetc(file)) != EOF){
+    arr[i] = c;
+    i++;
+  }
+  arr[i] = '\0';
+  return i;
+}
+
+int powerof(int base, int exponent){
+  int i, result;
+  if (exponent == 0){
+    return result = 1;
+  }
+  result = base;
+  for(i = 0; i < exponent - 1; i++){
+    result *= base;
+  }
+  return result;
+}
+
+int stringtoint(char number[], int length){
+  int j, result;
+  result = 0;
+  j = 0;
+  while(length - 1 >= 0){
+    result += ((int)number[length - 1] - '0') * (powerof(10, j));
+    j++;
+    length--;
+  }
+  return result;
+}
+
+int stringlength(char string[]){
+  int i = 0;
+  while(string[i] != '\0'){
+    i++;
+  }
+  return i;
+}
+
+int splitstr(char** dest, char line[], int index1, int index2){
+  int offset = index1;
+  while (line[index1] != ' ' && index1 < stringlength(line)){
+    index1++;
+  }
+  char* p = malloc(sizeof (char)*index1);
+  int i;
+  for(i = 0; i < index1 - offset; i++){
+    p[i] = line[i + offset];
+  }
+  p[i] = '\0';
+  dest[index2] = p;
+  return index1 + 1;
+}
+
+int countblanks(char line[]){
+  int i = 0;
+  int j = 0;
+  while(line[i] != '\0'){
+    if (line[i] == ' '){
+      j++;
+    }
+    i++;
+  }
+  return j;
+}
+
+void newlinetonullbyte(char string[]){
+  int i = 0;
+  while (string[i] != '\0'){
+    if (string[i] == '\n'){
+      string[i] = '\0';
+      break;
+    }
+    i++;
+  }
+}
+#endif