Fix warnings

README.txt

@@ -101,4 +101,6 @@ v0.01- very simple program to examine i7 feature clocking and running with speed
 coder: Abhishek Jaiantilal ([email protected]). And suggestions/help from multiple people, let me know if i am missing your contribution.
 contributor: raininja <[email protected]>
 contributor: Richard Hull <[email protected]>
-contributor: andareed
+contributor: andareed
+contributor: Alexander Riccio <[email protected]>
+

cpuinfo.c

@@ -141,7 +141,7 @@ void print_socket_information(struct cpu_socket_info* socket)
     for (i=0;i< socket->max_cpu ;i++) {
         assert(i < MAX_SK_PROCESSORS);
         if (socket->processor_num[i]!=-1) {
-            sprintf(socket_list,"%s%d,",socket_list,socket->processor_num[i]);
+            snprintf(socket_list, 200, "%s%d,",socket_list,socket->processor_num[i]);
         }
     }
     printf("Socket-%d [num of cpus %d physical %d logical %d] %s\n",socket->socket_num,socket->max_cpu,socket->num_physical_cores,socket->num_logical_cores,socket_list);

helper_functions.c

@@ -114,7 +114,7 @@ static inline void cpuid (unsigned int info, unsigned int *eax, unsigned int *eb
     if (edx) *edx = _edx;
 }
 
-static inline void  get_vendor (char *vendor_string)
+static inline void  get_vendor (char vendor_string[static 13])
 {
     //get vendor name
     unsigned int a, b, c, d;
@@ -166,7 +166,7 @@ double estimate_MHz ()
     *
     */
     struct timezone tz;
-    struct timeval tvstart, tvstop;
+    struct timeval tvstart = {0}, tvstop = {0};
     unsigned long long int cycles[2];		/* must be 64 bit */
     unsigned long long int microseconds;	/* total time taken */
 
@@ -242,7 +242,7 @@ uint64_t get_msr_value (int cpu, uint32_t reg, unsigned int highbit,
     int bits;
     *error_indx =0;
 
-    sprintf (msr_file_name, "/dev/cpu/%d/msr", cpu);
+    snprintf (msr_file_name, 64, "/dev/cpu/%d/msr", cpu);
     fd = open (msr_file_name, O_RDONLY);
     if (fd < 0)
     {
@@ -295,7 +295,7 @@ uint64_t set_msr_value (int cpu, uint32_t reg, uint64_t data)
     int fd;
     char msr_file_name[64];
 
-    sprintf (msr_file_name, "/dev/cpu/%d/msr", cpu);
+    snprintf (msr_file_name, 64, "/dev/cpu/%d/msr", cpu);
     fd = open (msr_file_name, O_WRONLY);
     if (fd < 0)
     {
@@ -369,7 +369,7 @@ void Print_Information_Processor(bool* nehalem, bool* sandy_bridge, bool* ivy_br
     */
     // somehow using strcmp or strncmp is crashing the app when using -O2, -O3 with gcc-4.7
     // (strncmp (vendor_string, "GenuineIntel",12) == 0)
-    // (strcmp (vendor_string, "GenuineIntel",12) == 0)
+    // (strcmp (vendor_string, "GenuineIntwel",12) == 0)
 
     if (strcmp (vendor_string, "GenuineIntel") == 0) {
     //if (equal_string) {
@@ -524,7 +524,7 @@ void Test_Or_Make_MSR_DEVICE_FILES()
         {
             //Try the Makedev script
             //sourced from MAKEDEV-cpuid-msr script in msr-tools
-            system ("msr_major=202; \
+            call_system ("msr_major=202; \
 							cpuid_major=203; \
 							n=0; \
 							while [ $n -lt 16 ]; do \
@@ -535,7 +535,7 @@ void Test_Or_Make_MSR_DEVICE_FILES()
 							done; \
 							");
             printf ("i7z DEBUG: modprobbing for msr\n");
-            system ("modprobe msr");
+            call_system ("modprobe msr");
         } else {
             printf ("i7z DEBUG: You DO NOT have root privileges, mknod to create device entries won't work out\n");
             printf ("i7z DEBUG: A solution is to run this program as root\n");
@@ -548,16 +548,22 @@ double cpufreq_info()
     //CPUINFO is wrong for i7 but correct for the number of physical and logical cores present
     //If Hyperthreading is enabled then, multiple logical processors will share a common CORE ID
     //http://www.redhat.com/magazine/022aug06/departments/tips_tricks/
-    system
+    call_system
     ("cat /proc/cpuinfo |grep MHz|sed 's/cpu\\sMHz\\s*:\\s//'|tail -n 1 > /tmp/cpufreq.txt");
 
-
     //Open the parsed cpufreq file and obtain the cpufreq from /proc/cpuinfo
-    FILE *tmp_file;
-    tmp_file = fopen ("/tmp/cpufreq.txt", "r");
-    char tmp_str[30];
-    fgets (tmp_str, 30, tmp_file);
+    FILE *tmp_file = fopen ("/tmp/cpufreq.txt", "r");
+    if(!tmp_file){
+        fprintf(stderr, "fopen /tmp/cpufreq.txt failed!\nerrno: %s\n", strerror(errno));
+    }
+
+    //zero initialize to prevent atof reading junk
+    char tmp_str[30] = {0};
+    const char *const fget_result = fgets (tmp_str, 30, tmp_file);
     fclose (tmp_file);
+    if(!fget_result){
+        fprintf(stderr, "fgets /tmp/cpufreq.txt failed!\n");
+    }
     return atof(tmp_str);
 }
 
@@ -680,7 +686,7 @@ void print_socket_information(struct cpu_socket_info* socket)
     for (i=0;i< socket->max_cpu ;i++) {
         assert(i < MAX_SK_PROCESSORS);
         if (socket->processor_num[i]!=-1) {
-            sprintf(socket_list,"%s%d,",socket_list,socket->processor_num[i]);
+            snprintf(socket_list, 200,"%s%d,",socket_list,socket->processor_num[i]);
         }
     }
     printf("Socket-%d [num of cpus %d physical %d logical %d] %s\n",socket->socket_num,socket->max_cpu,socket->num_physical_cores,socket->num_logical_cores,socket_list);

i7z.c

@@ -184,11 +184,11 @@ void logCpuFreq_single_ts(struct timespec  *value) //HW use timespec to avoid fl
 {
     //below when just logging
     if(prog_options.logging==1) {
-        fprintf(fp_log_file_freq,"%d.%.9d\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+        fprintf(fp_log_file_freq,"%ld.%.9ld\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
     }
     //below when appending
     if(prog_options.logging==2) {
-        fprintf(fp_log_file_freq,"%d.%.9d\t",value->tv_sec,value->tv_nsec);
+        fprintf(fp_log_file_freq,"%ld.%.9ld\t",value->tv_sec,value->tv_nsec);
     }
 }
 
@@ -264,20 +264,20 @@ void logCpuFreq_dual_ts(struct timespec  *value, int socket_num) //HW use timesp
     if(socket_num==0){
         //below when just logging
         if(prog_options.logging==1)
-            fprintf(fp_log_file_freq_1,"%d.%.9d\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+            fprintf(fp_log_file_freq_1,"%ld.%.9ld\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
 
         //below when appending
         if(prog_options.logging==2)
-             fprintf(fp_log_file_freq_1,"%d.%.9d\t",value->tv_sec,value->tv_nsec);
+             fprintf(fp_log_file_freq_1,"%ld.%.9ld\t",value->tv_sec,value->tv_nsec);
     }
     if(socket_num==1){
         //below when just logging
         if(prog_options.logging==1)
-            fprintf(fp_log_file_freq_2,"%d.%.9d\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+            fprintf(fp_log_file_freq_2,"%ld.%.9ld\n",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
 
         //below when appending
         if(prog_options.logging==2)
-             fprintf(fp_log_file_freq_2,"%d.%.9d\t",value->tv_sec,value->tv_nsec);
+             fprintf(fp_log_file_freq_2,"%ld.%.9ld\t",value->tv_sec,value->tv_nsec);
     }
 }
 
@@ -315,7 +315,7 @@ void logCpuCstates_single_ts(struct timespec  *value) //HW use timespec to avoid
 {
     //below when just logging
     if(prog_options.logging != 0) {
-        fprintf(fp_log_file_Cstates,"%d.%.9d",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+        fprintf(fp_log_file_Cstates,"%ld.%.9ld",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
     }
 }
 
@@ -366,27 +366,47 @@ void logCpuCstates_dual_ts(struct timespec  *value, int socket_num) //HW use tim
     if(socket_num==0){
         //below when just logging
         if(prog_options.logging != 0)
-            fprintf(fp_log_file_Cstates_1,"%d.%.9d",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+            fprintf(fp_log_file_Cstates_1,"%ld.%.9ld",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
     }
     if(socket_num==1){
         //below when just logging
         if(prog_options.logging != 0)
-            fprintf(fp_log_file_Cstates_2,"%d.%.9d",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
+            fprintf(fp_log_file_Cstates_2,"%ld.%.9ld",value->tv_sec,value->tv_nsec); //newline, replace \n with \t to get everything separated with tabs
     }
 }
 
-
+void call_system(const char* cmd)
+{
+    const int system_result = system(cmd);
+    //mkdir, mknod, return -1 on failure.
+    if(system_result == -1){
+        fprintf(stderr, "'%s' failed, it could not be created, or its status could not be retrieved. Result = -1\n", cmd);
+    }
+    else if(system_result == 127){
+        fprintf(stderr, "shell could not be executed for '%s'!. Result = 127\n", cmd);
+    }
+    //stty, modprobe, will only ever return EXIT_SUCCESS or EXIT_FAILURE.
+    else if(system_result == EXIT_FAILURE){
+        fprintf(stderr, "'%s' returned EXIT_FAILURE!\n", cmd);
+    }
+    //cat, tail, return values ">0" on failure.
+    //grep returns 1 if zero selected lines are found, and 2 if there's an error.
+    //sed can return 1, 2, and 4, on error
+    else if(system_result != EXIT_SUCCESS){
+        fprintf(stderr, "'%s' returned a non-zero value: %i\n", cmd, system_result);
+    }
+}
 
 
 void atexit_runsttysane()
 {
     printf("Quitting i7z\n");
-    system("stty sane");
+    call_system("stty sane");
 }
 
 void modprobing_msr()
 {
-    system("modprobe msr");
+    call_system("modprobe msr");
 }
 
 void init_ncurses()
@@ -468,7 +488,7 @@ int main (int argc, char **argv)
                 break;
             case 'l':
                 strncpy(log_file_name, optarg, MAX_FILENAME_LENGTH-3);
-                strcpy(log_file_name2, log_file_name);
+                strncpy(log_file_name2, log_file_name, MAX_FILENAME_LENGTH);
                 strcat(log_file_name2, "_%d");
                 CPU_FREQUENCY_LOGGING_FILE_single = log_file_name;
                 CPU_FREQUENCY_LOGGING_FILE_dual = log_file_name2;

i7z.h

@@ -166,6 +166,7 @@ void print_CPU_Heirarchy(struct cpu_heirarchy_info chi);
 int in_core_list(int ii,int* core_list);
 void Print_Version_Information();
 bool file_exists(char*);
+void call_system(const char* cmd);
 
 #define SET_ONLINE_ARRAY_MINUS1(online_cpus) {int iii;for(iii=0;iii<MAX_PROCESSORS;iii++) online_cpus[iii]=-1;}
 #define SET_ONLINE_ARRAY_PLUS1(online_cpus) {int iii;for(iii=0;iii<MAX_PROCESSORS;iii++) online_cpus[iii]=1;}

i7z_Dual_Socket.c

@@ -186,6 +186,8 @@ void print_i7z_socket(struct cpu_socket_info socket_0, int printw_offset, int PL
         RETURN_IF_TRUE(online_cpus[0]==-1);
 
         int IA32_FIXED_CTR_CTL = 909;	//38D
+
+        //FIXME: why isn't this value actually used??
         int IA32_FIXED_CTR_CTL_Value = get_msr_value (CPU_NUM, IA32_FIXED_CTR_CTL, 63, 0, &error_indx);
         SET_IF_TRUE(error_indx,online_cpus[0],-1);
         RETURN_IF_TRUE(online_cpus[0]==-1);
@@ -661,6 +663,7 @@ void print_i7z ()
     //int *core_list, core_list_size_phy, core_list_size_log;
 
     //iterator
+    //FIXME: why isn't this value actually used??
     int i;
 
     //turbo_mode enabled/disabled flag
@@ -705,6 +708,7 @@ void print_i7z ()
     TURBO_MODE = turbo_status ();
 
     //Flags and other things about HT.
+    //FIXME: why isn't this value actually used??
     int HT_ON;
     char HT_ON_str[30];
 

i7z_Single_Socket.c

@@ -204,13 +204,15 @@ void print_i7z_socket_single(struct cpu_socket_info socket_0, int printw_offset,
         char string_ptr1[200], string_ptr2[200];
 
         int IA32_PERF_GLOBAL_CTRL = 911;	//38F
+        //FIXME: why isn't this value actually used??
         int IA32_PERF_GLOBAL_CTRL_Value;
         IA32_PERF_GLOBAL_CTRL_Value =	get_msr_value (CPU_NUM, IA32_PERF_GLOBAL_CTRL, 63, 0, &error_indx);
         SET_IF_TRUE(error_indx,online_cpus[0],-1);
         RETURN_IF_TRUE(online_cpus[0]==-1);
 
         int IA32_FIXED_CTR_CTL = 909;	//38D
         int IA32_FIXED_CTR_CTL_Value;
+        //FIXME: why isn't this value actually used??
         IA32_FIXED_CTR_CTL_Value = get_msr_value (CPU_NUM, IA32_FIXED_CTR_CTL, 63, 0, &error_indx);
         SET_IF_TRUE(error_indx,online_cpus[0],-1);
         RETURN_IF_TRUE(online_cpus[0]==-1);
@@ -662,6 +664,7 @@ void print_i7z_single ()
     //int *core_list, core_list_size_phy, core_list_size_log;
 
     //iterator
+    //FIXME: why isn't this value actually used??
     int i;
 
     //turbo_mode enabled/disabled flag
@@ -714,6 +717,7 @@ void print_i7z_single ()
 
     //Flags and other things about HT.
     int HT_ON;
+    //FIXME: why isn't this value actually used??
     char HT_ON_str[30];
 
     int kk_1 = 11;