CPU core temperature Unity indicator

Question

Project now on Github: https://github.com/IanCaio/TempI

I just finished a working version of a small Unity indicator in C, that is supposed to take the output of the application "sensors" (sudo apt-get install sensors required) and uses it to monitor the temperature of the CPU.

It creates one indicator that has a small menu, with the name and authorship and a "Quit" button. Then it creates one indicator for each CPU core and displays the temperature of each, also changing the color of the icon depending on the temperature range.

I had to make some harder than average string manipulations and have to say I'm still not comfortable with it. Maybe I just have "buffer overflowphobia". Or maybe I just need to read string.h documentation until I know every function without needing to type man every time.

I tried to structure the program to make it really organized. Tried to divide it in many functions so it would be easier to review single tasks and if they are being done properly.

Both code blocks are a little large for a full review, but if you spot something that is bad practice on C I'd love to hear some feedback. Even if it's related only to a piece of the code. I'll review it myself since it just got of the oven and might have some hidden bugs (couldn't trace memory leaks on valgrind after some point because gtk and valgrind don't get along..).

Header:

#define TEMPI_MAX_CORES 4
#define TEMPI_MAX_CHARS 4096
#define TEMPI_COOL 50

//Function Prototypes
void TempI_Free_Everything(); //Frees memory
int TempI_Resolve_Executable_Path(); //Resolves executable path
int TempI_Read_Config(); //Reads config file and sets the variables on TempI_Main
void TempI_Set_Main_Indicator(); //Sets the main gtk app indicator
void TempI_Set_Core_Indicator(); //Sets the cores gtk app indicators
void TempI_Callback_Quit(); //Callback to quit program
int TempI_Number_Of_Cores(); //Returns the number of cores
gint TempI_Update(); //Updates the indicator
int TempI_Get_Core_Temperatures(); //Updates the temperatures values on every core

//Structures
typedef struct TempI_CPU_Core_Struct{
    //General info
    int Temperature; //Current CPU Core temperature
    int High_Value; //What is the CPU Core high temperature?
    int Critical_Value; //What is the CPU Core critical temperature?
    int Status; //0 = Cool, 1 = Above Cool, 2 = Hot, 3 = Close to critical!

    //Gtk objects (since each Core will have a icon+label on the panel, it's easier to keep them here)
    char *Gtk_Indicator_Name; //Name used as the Gtk_Indicator ID
    AppIndicator *Gtk_Indicator;
    char Gtk_Indicator_Label[20]; //Label with the current temperature
    GtkWidget *Gtk_Menu_Root; //Menu, necessary for displaying the indicator
    GtkWidget *Gtk_Menu_Root_Description; //Just a description of the core being monitored (and exact temperature)
} TempI_CPU_Core;

typedef struct TempI_Main_Struct{
    //General use variables
    int Delay; //Delay between each 'sensors' process call
    int Log; //Should we write to a log file?
    char *LogFileName; //The log file name
    FILE *LogFile; //Log file handler
    char *ExecutablePath; //Executable full path

    //CPU Cores
    TempI_CPU_Core Core[TEMPI_MAX_CORES];
    int Cores_Counter; //How many cores are being monitored?
    char *Gtk_Core_Icon_Path[4]; //Path to different icons for the CPU indicator

    //Gtk objects
    char *Gtk_Indicator_Icon_Path; //Path to main indicator icon
    AppIndicator *Gtk_Indicator;
    GtkWidget *Gtk_Menu_Root;
    GtkWidget *Gtk_Menu_Root_Description;
    GtkWidget *Gtk_Menu_Root_Separator;
    GtkWidget *Gtk_Menu_Root_Quit;
} TempI_Main_t;

Main:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>

#include<gtk/gtk.h>
#include<libappindicator/app-indicator.h>

#include "debugger.h"
#include "TempI.h"

TempI_Main_t TempI_Main;

int main(int argc, char **argv){
    //Initialize TempI_Main
        TempI_Main.Delay=2;
        TempI_Main.Log=0;
        TempI_Main.LogFileName=NULL;
        TempI_Main.LogFile=NULL;
        TempI_Main.ExecutablePath=NULL;
        TempI_Main.Cores_Counter=0;
        TempI_Main.Gtk_Indicator_Icon_Path=NULL;
        for(int i=0;i<4;i++){
            TempI_Main.Gtk_Core_Icon_Path[i]=NULL;
        }

    //Executable path is needed for reading the configuration file and loading icons for the indicator
    D_check(TempI_Resolve_Executable_Path()==0,"Couldn't resolve executable path.");

    D_check(TempI_Read_Config()==0,"Couldn't read configuration file.");

    TempI_Main.Cores_Counter=TempI_Number_Of_Cores();
    D_debug("Number of Cores: %u.",TempI_Main.Cores_Counter);

    //LogFile
    if(TempI_Main.Log==1){
        TempI_Main.LogFile=fopen(TempI_Main.LogFileName,"w");
    }

    //Starts gtk
    gtk_init(&argc,&argv);

    TempI_Set_Main_Indicator();
    TempI_Set_Core_Indicator();

    //Update routine will be called as a timeout on Glib
    g_timeout_add(TempI_Main.Delay*1000,TempI_Update,NULL);

    gtk_main();

    TempI_Free_Everything();
    return 0;

error:
    TempI_Free_Everything();
    return -1;
}

gint TempI_Update(){
    D_check(TempI_Get_Core_Temperatures()==0,"Couldn't update core temperatures.");

    //Used to make the log file time stamps
    time_t Rawtime;
    struct tm *TimeStruct;
    char *TimeStamp;

    time(&Rawtime);
    TimeStruct=localtime(&Rawtime);
    TimeStamp=asctime(TimeStruct);

    if(TempI_Main.Log==1){
        fprintf(TempI_Main.LogFile,"%s",TimeStamp);
    }

    //Updates status and sets the appropriate icon depending on the core status
    for(int i=0; i<TempI_Main.Cores_Counter; i++){
        int TempRange=TempI_Main.Core[i].High_Value-TEMPI_COOL;
        int Status2Temp=TempI_Main.Core[i].High_Value-(TempRange/3);
        int Status1Temp=TempI_Main.Core[i].High_Value-(2*TempRange/3);

        D_debug("Status2 Temp: %u\nStatus1 Temp: %u",Status2Temp,Status1Temp);

        if(TempI_Main.Core[i].Temperature>TempI_Main.Core[i].High_Value){
            TempI_Main.Core[i].Status=3;
        } else if(TempI_Main.Core[i].Temperature>Status2Temp && TempI_Main.Core[i].Temperature<=TempI_Main.Core[i].High_Value){
            TempI_Main.Core[i].Status=2;
        } else if(TempI_Main.Core[i].Temperature>Status1Temp && TempI_Main.Core[i].Temperature<=Status2Temp){
            TempI_Main.Core[i].Status=1;
        } else {
            TempI_Main.Core[i].Status=0;
        }

        snprintf(TempI_Main.Core[i].Gtk_Indicator_Label,20,"Core %u: %uºC",i,TempI_Main.Core[i].Temperature);

        app_indicator_set_icon(TempI_Main.Core[i].Gtk_Indicator,TempI_Main.Gtk_Core_Icon_Path[TempI_Main.Core[i].Status]);
        app_indicator_set_label(TempI_Main.Core[i].Gtk_Indicator,TempI_Main.Core[i].Gtk_Indicator_Label,NULL);

        if(TempI_Main.Log==1){
            fprintf(TempI_Main.LogFile,"Core %u:\n\tTemperature=%uºC\n\tHigh Temperature=%uºC\n\tCritical Temperature=%uºC\n",
                    i,TempI_Main.Core[i].Temperature,TempI_Main.Core[i].High_Value,TempI_Main.Core[i].Critical_Value);
        }
    }

    if(TempI_Main.Log==1){
        fflush(TempI_Main.LogFile);
    }

    return TRUE;
error:
    return FALSE;
}

int TempI_Get_Core_Temperatures(){
    int NoC=0; //Counter to check which core are we reading output from
    FILE *sensors_proc;
    char sensor_line[TEMPI_MAX_CHARS];

    sensors_proc=popen("sensors","r");
    D_check_pointer(sensors_proc);

    while(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
        if(strncmp(sensor_line,"Adapter: Virtual device",23)==0){
            //Ignore virtual device
            for(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc); sensor_line[0]!='\n';fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
                //Ignored
            }
        } else if(strncmp(sensor_line,"Adapter:",8)==0){
            //Extract temperature, high temperature and critical temperature for each core
            //Output will be something like:
            //Core 0:       +57.0°C  (high = +95.0°C, crit = +105.0°C)
            //Core 2:       +54.0°C  (high = +95.0°C, crit = +105.0°C)

            NoC=0;

            for(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc); sensor_line[0]!='\n';fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
                char *string_pointer;
                char Temperature_String[10]; //Doesn't have to be that big, but just in case your CPU is as hot as the sun...
                char HighTemperature_String[10];
                char CriticalTemperature_String[10];

                D_debug("Sensors line: %s",sensor_line);

                //Goes to the first +, right behind the temperature
                string_pointer=strchr(sensor_line,'+');
                D_check(string_pointer!=NULL,"Invalid sensors input.");

                string_pointer++; //Pointing at the first digit of temperature

                for(int i=0; i<9 && *string_pointer!='.'; i++){
                    Temperature_String[i]=*string_pointer;
                    Temperature_String[i+1]='\0'; //Making sure we end the string properly
                    string_pointer++;
                }
                //Now string_pointer is at the decimals.
                while(*string_pointer!='+'){                    
                    string_pointer++;
                }

                string_pointer++; //Pointing at the first digit of high temperature

                for(int i=0; i<9 && *string_pointer!='.'; i++){
                    HighTemperature_String[i]=*string_pointer;
                    HighTemperature_String[i+1]='\0'; //Making sure we end the string properly
                    string_pointer++;
                }
                //Now string_pointer is at the decimals.
                while(*string_pointer!='+'){                    
                    string_pointer++;
                }

                string_pointer++; //Pointing at the first digit of high temperature

                for(int i=0; i<9 && *string_pointer!='.'; i++){
                    CriticalTemperature_String[i]=*string_pointer;
                    CriticalTemperature_String[i+1]='\0'; //Making sure we end the string properly
                    string_pointer++;
                }

                TempI_Main.Core[NoC].Temperature=atoi(Temperature_String);
                TempI_Main.Core[NoC].High_Value=atoi(HighTemperature_String);
                TempI_Main.Core[NoC].Critical_Value=atoi(CriticalTemperature_String);

                D_debug("Temperature String: %s",Temperature_String);
                D_debug("HighTemperature String: %s",HighTemperature_String);
                D_debug("CriticalTemperature String: %s",CriticalTemperature_String);

                D_debug("Values on Core: T: %u - H: %u - C: %u",TempI_Main.Core[NoC].Temperature,TempI_Main.Core[NoC].High_Value,TempI_Main.Core[NoC].Critical_Value);

                NoC++;
            }
        }
    }

    pclose(sensors_proc);

    return 0;
error:
    if(sensors_proc){
        pclose(sensors_proc);
    }

    return -1;
}

//This function sets the Gtk widgets and app indicator for each core
void TempI_Set_Core_Indicator(){
    //Resolve Core states icon paths
    size_t IconPathSize;
    //ExecutablePath + "/icons/green-icon.png" + '\0' = ExecutablePath + 22 chars
    IconPathSize=strlen(TempI_Main.ExecutablePath)+22;
    TempI_Main.Gtk_Core_Icon_Path[0]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[0],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[0],"/icons/green-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[0]));
    //ExecutablePath + "/icons/yellow-icon.png" + '\0' = ExecutablePath + 23 chars
    IconPathSize=strlen(TempI_Main.ExecutablePath)+23;
    TempI_Main.Gtk_Core_Icon_Path[1]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[1],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[1],"/icons/yellow-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[1]));
    //ExecutablePath + "/icons/orange-icon.png" + '\0' = ExecutablePath + 23 chars
    IconPathSize=strlen(TempI_Main.ExecutablePath)+23;
    TempI_Main.Gtk_Core_Icon_Path[2]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[2],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[2],"/icons/orange-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[2]));
    //ExecutablePath + "/icons/red-icon.png" + '\0' = ExecutablePath + 20 chars
    IconPathSize=strlen(TempI_Main.ExecutablePath)+20;
    TempI_Main.Gtk_Core_Icon_Path[3]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[3],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[3],"/icons/red-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[3]));

    D_debug("Core Icon Paths:\n%s\n%s\n%s\n%s",TempI_Main.Gtk_Core_Icon_Path[0],TempI_Main.Gtk_Core_Icon_Path[1],TempI_Main.Gtk_Core_Icon_Path[2],TempI_Main.Gtk_Core_Icon_Path[3]);

    for(int i=0; i<TempI_Main.Cores_Counter; i++){
        TempI_Main.Core[i].Gtk_Menu_Root=gtk_menu_new();

        //The core name ("Core " + number)
        char IndicatorName[TEMPI_MAX_CHARS];
        snprintf(IndicatorName, TEMPI_MAX_CHARS,"TempI_Core %u",i);

        TempI_Main.Core[i].Gtk_Indicator_Name=strdup(IndicatorName);

        TempI_Main.Core[i].Gtk_Menu_Root_Description=gtk_menu_item_new_with_label("Nothing to see here!");
        gtk_menu_append(GTK_MENU(TempI_Main.Core[i].Gtk_Menu_Root),TempI_Main.Core[i].Gtk_Menu_Root_Description);
        gtk_widget_set_sensitive(GTK_WIDGET(TempI_Main.Core[i].Gtk_Menu_Root_Description),FALSE);
        gtk_widget_show(TempI_Main.Core[i].Gtk_Menu_Root_Description);

        TempI_Main.Core[i].Gtk_Indicator=app_indicator_new(TempI_Main.Core[i].Gtk_Indicator_Name,TempI_Main.Gtk_Core_Icon_Path[0],APP_INDICATOR_CATEGORY_APPLICATION_STATUS);
        app_indicator_set_status(TempI_Main.Core[i].Gtk_Indicator, APP_INDICATOR_STATUS_ACTIVE);

        app_indicator_set_menu(TempI_Main.Core[i].Gtk_Indicator,GTK_MENU(TempI_Main.Core[i].Gtk_Menu_Root));
    }
}

//This function calls 'sensors' to find out how many cores are being monitored
//return -1 = error
int TempI_Number_Of_Cores(){
    int NoC=0;

    FILE *sensors_proc;
    char sensor_line[TEMPI_MAX_CHARS];

    sensors_proc=popen("sensors","r");
    D_check_pointer(sensors_proc);

    while(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
        if(strncmp(sensor_line,"Adapter: Virtual device",23)==0){
            //Ignore virtual device
            for(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc); sensor_line[0]!='\n';fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
                //Ignored
            }
        } else if(strncmp(sensor_line,"Adapter:",8)==0){
            //Count lines
            NoC=0;
            for(fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc); sensor_line[0]!='\n';fgets(sensor_line,TEMPI_MAX_CHARS,sensors_proc)){
                //Ignoring information, counting cores
                NoC++;
            }
        }
    }

    pclose(sensors_proc);

    return NoC;

error:
    return -1;
}

//Callback to quit the program (Gtk style)
void TempI_Callback_Quit(){
    gtk_main_quit();
}

//This function sets the main indicator
void TempI_Set_Main_Indicator(){
    TempI_Main.Gtk_Menu_Root=gtk_menu_new();

    TempI_Main.Gtk_Menu_Root_Description=gtk_menu_item_new_with_label("TempI - version 0.1a\nAuthor: Ian");
    gtk_menu_append(GTK_MENU(TempI_Main.Gtk_Menu_Root),TempI_Main.Gtk_Menu_Root_Description);
    gtk_widget_set_sensitive(GTK_WIDGET(TempI_Main.Gtk_Menu_Root_Description),FALSE);
    gtk_widget_show(TempI_Main.Gtk_Menu_Root_Description);

    TempI_Main.Gtk_Menu_Root_Separator=gtk_separator_menu_item_new();
    gtk_menu_append(GTK_MENU(TempI_Main.Gtk_Menu_Root),TempI_Main.Gtk_Menu_Root_Separator);
    gtk_widget_show(TempI_Main.Gtk_Menu_Root_Separator);

    TempI_Main.Gtk_Menu_Root_Quit=gtk_menu_item_new_with_label("Quit");
    gtk_menu_append(GTK_MENU(TempI_Main.Gtk_Menu_Root),TempI_Main.Gtk_Menu_Root_Quit);
    gtk_signal_connect(GTK_OBJECT(TempI_Main.Gtk_Menu_Root_Quit),"activate",GTK_SIGNAL_FUNC(TempI_Callback_Quit),NULL);
    gtk_widget_show(TempI_Main.Gtk_Menu_Root_Quit);

    //Get the main icon path
    //ExecutablePath + "/icons/menu-icon.png" + '\0' = ExecutablePath + 21 chars
    size_t IconPathSize=strlen(TempI_Main.ExecutablePath)+21;
    TempI_Main.Gtk_Indicator_Icon_Path=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Indicator_Icon_Path,TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Indicator_Icon_Path,"/icons/menu-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Indicator_Icon_Path));

    D_debug("Main Indicator Icon Path: %s",TempI_Main.Gtk_Indicator_Icon_Path);

    TempI_Main.Gtk_Indicator=app_indicator_new("TempI_Main",TempI_Main.Gtk_Indicator_Icon_Path,APP_INDICATOR_CATEGORY_APPLICATION_STATUS);
    app_indicator_set_status(TempI_Main.Gtk_Indicator, APP_INDICATOR_STATUS_ACTIVE);

    app_indicator_set_menu(TempI_Main.Gtk_Indicator, GTK_MENU(TempI_Main.Gtk_Menu_Root));
}

//This function is just a clean up function to make heap memory leaks less likely
//IMPORTANT: If a memory portion was free'd, the pointer MUST be null or we will
//run into "double free" issues.
void TempI_Free_Everything(){
    if(TempI_Main.LogFile){
        fclose(TempI_Main.LogFile);
        TempI_Main.LogFile=NULL;
    }

    for(int i=0;i<TEMPI_MAX_CORES;i++){
        if(TempI_Main.Core[i].Gtk_Indicator_Name){
            free(TempI_Main.Core[i].Gtk_Indicator_Name);
            TempI_Main.Core[i].Gtk_Indicator_Name=NULL;
        }
    }

    for(int i=0;i<4;i++){
        if(TempI_Main.Gtk_Core_Icon_Path[i]){
            free(TempI_Main.Gtk_Core_Icon_Path[i]);
            TempI_Main.Gtk_Core_Icon_Path[i]=NULL;
        }
    }

    if(TempI_Main.Gtk_Indicator_Icon_Path){
        free(TempI_Main.Gtk_Indicator_Icon_Path);
        TempI_Main.Gtk_Indicator_Icon_Path=NULL;
    }

    if(TempI_Main.ExecutablePath){
        free(TempI_Main.ExecutablePath);
        TempI_Main.ExecutablePath=NULL;
    }

    if(TempI_Main.LogFileName){
        free(TempI_Main.LogFileName);
        TempI_Main.LogFileName=NULL;
    }
}

//This function resolves the executable current path. It's necessary mostly for the
//libappindicator icon loading, since it doesn't seem to be able to resolve incomplete paths.
//Returns 0 if all went fine, 1 if it didn't.
int TempI_Resolve_Executable_Path(){
    FILE *proc_maps;
    char proc_maps_line[TEMPI_MAX_CHARS];   //Is it better to have this variable on the stack?
                                            //Or should I allocate it on the heap? (overhead of asking the system for a place on
                                            //the heap and all the allocation routines)
    char *string_pointer;

    proc_maps=fopen("/proc/self/maps","r");
    D_check_pointer(proc_maps);

    //Only first line is necessary
    fgets(proc_maps_line,TEMPI_MAX_CHARS,proc_maps);
    fclose(proc_maps);

    //It will look something like this:
    //00400000-0040c000 r-xp 00000000 fc:01 18612248                           /bin/cat

    //Points to the last character, the newline
    string_pointer=strchr(proc_maps_line,'\n');
    *string_pointer='\0'; //Replaces newline with null character

    //While string_pointer doesn't point the a space (meaning the path is over)
    while(*string_pointer != ' '){
        string_pointer--;
    }

    //Now string pointer is pointing to the first character in the path
    string_pointer++;

    //Allocates memory long enough to hold the path and the null terminator '\0'
    size_t path_size=strlen(string_pointer)+1;
    TempI_Main.ExecutablePath=malloc(path_size);

    D_check_pointer(TempI_Main.ExecutablePath);

    strncpy(TempI_Main.ExecutablePath,string_pointer,path_size);

    //Remove application name
    string_pointer=strrchr(TempI_Main.ExecutablePath,'/'); //Finds last occurence of '/'
    *string_pointer='\0'; //The application name is still on the variable, but is ignored because of the null terminator

    D_debug("Executable Path: %s",TempI_Main.ExecutablePath);

    return 0;

error:
    return 1;
}

//Reads configuration file and sets the program according to it
//Returns 0 if all went fine, 1 if something went wrong.
int TempI_Read_Config(){
    FILE *config_file;
    char config_path[TEMPI_MAX_CHARS];
    char config_line[TEMPI_MAX_CHARS];

    strncpy(config_path,TempI_Main.ExecutablePath,TEMPI_MAX_CHARS);
    strncat(config_path,"/Config/TempI.config",TEMPI_MAX_CHARS-strlen(config_path));

    D_debug("Configuration file path: %s",config_path);

    config_file=fopen(config_path,"r");

    D_check_pointer(config_file);

    char *string_pointer;

    while(fgets(config_line,TEMPI_MAX_CHARS,config_file)){
        if(strncmp(config_line,"TIMER=",6)==0){
            string_pointer=strchr(config_line,'=');
            string_pointer++;

            TempI_Main.Delay=atoi(string_pointer);
        } else if(strncmp(config_line,"LOG=",4)==0){
            string_pointer=strchr(config_line,'=');
            string_pointer++;

            if(strncmp(string_pointer,"TRUE",4)==0){
                TempI_Main.Log=1;
            } else {
                TempI_Main.Log=0;
            }
        } else if(strncmp(config_line,"LOGFILE=",8)==0){
            //Removes newline
            string_pointer=strrchr(config_line,'\n');
            *string_pointer='\0';

            string_pointer=strchr(config_line,'=');
            string_pointer++;

            //Size of file name + 1 to store \0
            size_t FileNameSize=strlen(string_pointer)+1;
            TempI_Main.LogFileName=malloc(FileNameSize);
            strncpy(TempI_Main.LogFileName,string_pointer,FileNameSize);
        }
    }

    //Default value for delay if a improper value is given
    if(TempI_Main.Delay<1 || TempI_Main.Delay>5){
        TempI_Main.Delay=2;
    }

    D_debug("Logfile name: %s",TempI_Main.LogFileName);
    D_debug("Delay: %u",TempI_Main.Delay);
    D_debug("Log: %u",TempI_Main.Log);

    fclose(config_file);
    return 0;

error:
    return 1;
}

The "debugger.h" file just contains some macros used for debugging, inspired by Zed's from "LearnCTheHardWay", but with some small changes I found pertinent:

#ifndef __debugger_h__
#define __debugger_h__

#include <stdio.h>
#include <errno.h>
#include <string.h>

#ifdef NO_DEBUG
#define D_debug(M, ...)
#else
#define D_debug(M, ...) fprintf(stderr, "DEBUG %s:%d: " M "\n", __FILE__, __LINE__, ##__VA_ARGS__)
#endif

#define D_check_errno() (errno==0 ? "None" : strerror(errno))

#define D_log_error(M, ...) fprintf(stderr, "[ERROR] (%s:%d: errno: %s) " M "\n", __FILE__, __LINE__, D_check_errno(), ##__VA_ARGS__)

#define D_log_warning(M, ...) fprintf(stderr, "[WARNING] (%s:%d: errno: %s) " M "\n", __FILE__, __LINE__, D_check_errno(), ##__VA_ARGS__)

#define D_log_info(M, ...) fprintf(stderr, "[INFO] (%s:%d:) " M "\n", __FILE__, __LINE__, ##__VA_ARGS__)

#define D_check(A, M, ...) do {if(!(A)) {D_log_error(M, ##__VA_ARGS__); errno=0; goto error;}} while(0)

#define D_sentinel(M, ...) do {D_log_error(M, ##__VA_ARGS__); errno=0; goto error;} while(0)

#define D_check_pointer(A) D_check((A), "Pointer " #A " is NULL.")

#define D_check_debug(A, M, ...) do {if(!(A)) {D_debug(M, ##__VA_ARGS__); errno=0; goto error;}} while(0)

#endif

Answer 1

This definitely looks like a real project that someone can use or expand. Have you put it on GitHub?

The symbolic constants defined in TempI.h are following the C Standard.

Some general considerations, while the most common numbers of cores in a CPU chip are currently 1, 2 or 4, there can also be larger powers of 2 such as 8, 16 or 32. Rather than using a fixed size array, it might be better to get the number of cores at run time and then create an array of the appropriate size. The number of icons for cores could remain the same as long as a modulo is utilized when indexing the array of core icons.

The size of the program indicates that modularity might be a good thing, consider breaking the single C file into multiple files and using a makefile to build the program. If the program gets posted on GitHub, a makefile will be necessary.

There is no test in the program to see if the number of cores exceeds TEMPI_MAX_CORES, this can lead to unknown results/errors in the program.

Hiding Goto
The macros in debugger.h hide the use of goto. While the use of goto in modern languages is frowned upon there are legitimate uses for it, such as error handling such as this code is using it. Hiding goto's is really a bad practice, because a goto changes the flow of the logic and the flow of the code. When a goto is hidden in a macro it is VERY difficult to maintain and debug code, in fact almost impossible unless someone finds the label. The fact that the goto is hidden means that a programmer may overlook labels if they are scanning the code quickly.

You probably should look into setjmp() and longjmp() for error conditions and error handling. While longjmp() is a form of goto, it is a goto that jumps to a location set previously by setjmp() and is sometimes used to return program flow to main() to handle any error conditions in a consistent manner (deallocation of resources such as freeing allocated memory and closing files prior to exiting). It is discussed by this question on stackoverflow.com.

Global Variables
The code contains :

TempI_Main_t TempI_Main;

int main(int argc, char **argv){
...
}

indicating that the variable TempI_Main is a global variable.

Many of us are taught either in school or in our first year at work not to use global variables. When one uses global variables all changes to the variables that are global are side effects. The down side of side effects are that they can happen anywhere, and are very hard to trace. This makes the code harder to write, maintain, and especially debug.

You can read more about why global variables are considered bad at this question on stackoverflow.com, and a decent discussion of when to use global variables on programmers.com. If you search for global variables on Google, you will find more diatribes against them.

In the case of this code, TempI_Main should probably be declared in main() and then passed as a parameter to all the functions that use it. This would be more maintainable and easier to debug.

Inconsistent Indentation
In main() The initialization of the TempI_MAIN structure is indented more than the rest of the code. This is unusual to say the least. Try to keep all of the indentation at the proper level. There are alternates of initializing structs as well.

One such alternate method is:

    TempI_Main TempI_Main = {
        .Delay=2;
        .Log=0;
        .LogFileName=NULL;
        .LogFile=NULL;
        .ExecutablePath=NULL;
        .Cores_Counter=0;
        .Gtk_Indicator_Icon_Path=NULL;
    }

this may give you the indentation you were seeking.

Inconsisten use of Symbolic Constants and Magic Numbers
While there are symbolic constants defined in TempI.h there are places where they aren't used that they could be. In TempI.h there is the following code:

#define TEMPI_MAX_CORES 4
#define TEMPI_MAX_CHARS 4096
#define TEMPI_COOL 50

and then

    //CPU Cores
    TempI_CPU_Core Core[TEMPI_MAX_CORES];
    int Cores_Counter; //How many cores are being monitored?
    char *Gtk_Core_Icon_Path[4]; //Path to different icons for the CPU indicator

It's quite possible that Gtk_Core_Icon_Path[0] should be Gtk_Core_Icon_Path[TEMPI_MAX_CORES] and that the following code in main()

        for(int i=0;i<4;i++){
            TempI_Main.Gtk_Core_Icon_Path[i]=NULL;
        }

should be

        for(int i = 0; i < TEMPI_MAX_CORES; i++){
            TempI_Main.Gtk_Core_Icon_Path[i] = NULL;
        }

Please note the use of spaces in the for loop, it will help make the code easier to read and maintain.

It's not clear in the code where the array of TempI_CPU_Core_Struct is initialized. Perhaps the previous loop should be

        for(int i = 0; i < TEMPI_MAX_CORES; i++){
            TempI_Main.Gtk_Core_Icon_Path[i] = NULL;
            memset(&TempI_Main.Core[i], 0, sizeof(TempI_CPU_Core_Struct));
        }

The call to memset() above initializes the entire struct to zero, any pointers are then turned into NULL.

Based on TempI_Main.Core being an array, it might be better to call it TempI_Main.Cores to indicate multiple elements.

Function Size and Complexity
Several of the functions are large or very complex, and perhaps they should be broken up into smaller functions. One example the following code in TempI_Update() deserves to be in it's own function due to the comlexit:

        int TempRange=TempI_Main.Core[i].High_Value-TEMPI_COOL;
        int Status2Temp=TempI_Main.Core[i].High_Value-(TempRange/3);
        int Status1Temp=TempI_Main.Core[i].High_Value-(2*TempRange/3);

        if(TempI_Main.Core[i].Temperature>TempI_Main.Core[i].High_Value){
            TempI_Main.Core[i].Status=3;
        } else if(TempI_Main.Core[i].Temperature>Status2Temp && TempI_Main.Core[i].Temperature<=TempI_Main.Core[i].High_Value){
            TempI_Main.Core[i].Status=2;
        } else if(TempI_Main.Core[i].Temperature>Status1Temp && TempI_Main.Core[i].Temperature<=Status2Temp){
            TempI_Main.Core[i].Status=1;
        } else {
            TempI_Main.Core[i].Status=0;
        }

This would make the code easier to maintain and debug. It would also allow the code to deal with the TempI_CPU_Core_Struct directly and not require the indexing of the Core Array.

Another example of code that should be put into a function in TempI_Set_Core_Indicator():

    size_t IconPathSize;

    IconPathSize=strlen(TempI_Main.ExecutablePath)+22;
    TempI_Main.Gtk_Core_Icon_Path[0]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[0],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[0],"/icons/green-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[0]));

    IconPathSize=strlen(TempI_Main.ExecutablePath)+23;
    TempI_Main.Gtk_Core_Icon_Path[1]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[1],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[1],"/icons/yellow-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[1]));

    IconPathSize=strlen(TempI_Main.ExecutablePath)+23;
    TempI_Main.Gtk_Core_Icon_Path[2]=malloc(IconPathSize);        strncpy(TempI_Main.Gtk_Core_Icon_Path[2],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[2],"/icons/orange-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[2]));

    IconPathSize=strlen(TempI_Main.ExecutablePath)+20;
    TempI_Main.Gtk_Core_Icon_Path[3]=malloc(IconPathSize);
    strncpy(TempI_Main.Gtk_Core_Icon_Path[3],TempI_Main.ExecutablePath,IconPathSize);
    strncat(TempI_Main.Gtk_Core_Icon_Path[3],"/icons/red-icon.png",IconPathSize-strlen(TempI_Main.Gtk_Core_Icon_Path[3]));

The appropriate sub-function here might be:

char *Allocate_Gtk_Core_Path(char *Base_Icon_Path, char *Executable_Path) {
    char *IconPath = NULL;
    size_t BaseIconPathSize = strlen(Base_Icon_Path);
    size_t IconPathSize = BaseIconPathSize + strlen(Executable_Path) + 1; // 1 for null termination of string

    IconPath = malloc(IconPathSize);
    if (IconPath != NULL) {
        strncpy(IconPath, Executable_Path, IconPathSize);
        strncat(IconPath, Base_Icon_Path, BaseIconPathSize);

        strncat(IconPath, Base_Icon_Path, BaseIconPathSize);
    }

    return IconPath;
}

Code in TempI_Set_Core_Indicator() is replaced with:

    char *IconBasePaths[] = {
        "/icons/green-icon.png",
        "/icons/yellow-icon.png",
        "/icons/orange-icon.png"
        "/icons/red-icon.png"
    };

    for (int i = 0; i < TempI_Main.Cores_Counter; i++) {
        TempI_Main.Gtk_Core_Icon_Path[i] = Allocate_Gtk_Core_Path(TempI_Main.ExecutablePath, IconBasePaths[i]);
        if (TempI_Main.Gtk_Core_Icon_Path[i] == NULL) {
            // print message here
            goto error;
        }
    }

As you can see the code is somewhat easier to read and debug.

A number of Software Engineering / Programming Principles apply here, they are the Single Responsibility Principle, the DRY Principle and the KISS Principle.

The Single Responsibility Principle is that each function should do one thing and one thing only.

One example of the Single Responsibility Principle is the function Allocate_Gtk_Core_Path().

The DRY principle is the Don't Repeat Yourself principle, any time you have code that repeats itself such as the second example, you should move the code to a function so that it only needs to be written and debugged once.

An example of DRY code is the replacement for loop.

The KISS principle is Keep It Simple. The less complex a function is, the easier it is to write, read, debug and maintain.

Always Test the Return Value of Malloc
The function malloc() allocates memory. There are situations where the call to malloc() may fail such as if there is no memory available, or no memory block large enough available. The function malloc() returns NULL if the memory allocation failed. The pointer to memory that is receiving the code from the call to malloc() should always be tested after the allocation to see if it is a valid memory location.

The following code should be augmented:

    TempI_Main.Gtk_Core_Icon_Path[0]=malloc(IconPathSize);

it should become

    TempI_Main.Gtk_Core_Icon_Path[0] = malloc(IconPathSize);
    if (TempI_Main.Gtk_Core_Icon_Path[0] == NULL) {
        fprintf(stderr, "malloc failed in TempI_Set_Core_Indicator()");
        goto error;
    }

Answer 2

1. Units. Physical quantities like temperature benefit with documented units. Your header file should state them like degrees F, tenths of degrees C, etc.

   int Temperature; //Current CPU Core temperature
   int High_Value; //What is the CPU Core high temperature?
   

2. Avoid magic numbers like 20.

   // snprintf(TempI_Main.Core[i].Gtk_Indicator_Label,20,"Core %u: %uºC",i,...);
   snprintf(TempI_Main.Core[i].Gtk_Indicator_Label,
       sizeof TempI_Main.Core[i].Gtk_Indicator_Label,"Core %u: %uºC",i,...);
   

3. Why 20 in char Gtk_Indicator_Label[20];? Code uses no check on snprintf(), so insure the size is sufficient, even with unreasonable unsigned values.

     #define UINT_STR10_SIZE (sizeof(unsigned)*CHAR_BIT*3/10 + 2)
     #define LABEL_FORMAT "Core %u: %uºC"
     #define LABEL_N (sizeof LABEL_FORMAT + 2*UINT_STR10_SIZE)
     char Gtk_Indicator_Label[LABEL_N];
     snprintf(TempI_Main.Core[i].Gtk_Indicator_Label,
         sizeof TempI_Main.Core[i].Gtk_Indicator_Label, LABEL_FORMAT, i, ...);
   

4. Style: excessive long lines. Example:

   // D_debug("Core Icon Paths:\n%s\n%s\n%s\n%s",TempI_Main.Gtk_Core_Icon_Path[0],TempI_Main.Gtk_Core_Icon_Path[1],TempI_Main.Gtk_Core_Icon_Path[2],TempI_Main.Gtk_Core_Icon_Path[3]);
   D_debug("Core Icon Paths:\n%s\n%s\n%s\n%s",
       TempI_Main.Gtk_Core_Icon_Path[0],TempI_Main.Gtk_Core_Icon_Path[1],
       TempI_Main.Gtk_Core_Icon_Path[2],TempI_Main.Gtk_Core_Icon_Path[3]);
   

Moving to wiki