From 9db9842aeabe76f78bfda67fac89b75a531bb0b7 Mon Sep 17 00:00:00 2001 From: Erik van der Zalm Date: Wed, 14 May 2014 21:59:48 +0200 Subject: [PATCH] Fixed error found by the free coverity tool (https://scan.coverity.com/) =================================================== Hi, Please find the latest report on new defect(s) introduced to ErikZalm/Marlin found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 15 of 15 defect(s) ** CID 59629: Unchecked return value (CHECKED_RETURN) /Marlin_main.cpp: 2154 in process_commands()() ** CID 59630: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) /Applications/Arduino.app/Contents/Resources/Java/hardware/arduino/cores/arduino/Tone.cpp: 319 in tone(unsigned char, unsigned int, unsigned long)() ** CID 59631: Missing break in switch (MISSING_BREAK) /Marlin_main.cpp: 1187 in process_commands()() ** CID 59632: Missing break in switch (MISSING_BREAK) /Marlin_main.cpp: 1193 in process_commands()() ** CID 59633: Out-of-bounds write (OVERRUN) /temperature.cpp: 914 in disable_heater()() ** CID 59634: Out-of-bounds write (OVERRUN) /temperature.cpp: 913 in disable_heater()() ** CID 59635: Out-of-bounds read (OVERRUN) /temperature.cpp: 626 in analog2temp(int, unsigned char)() ** CID 59636: Out-of-bounds read (OVERRUN) /temperature.cpp: 620 in analog2temp(int, unsigned char)() ** CID 59637: Out-of-bounds write (OVERRUN) /temperature.cpp: 202 in PID_autotune(float, int, int)() ** CID 59638: Out-of-bounds read (OVERRUN) /temperature.cpp: 214 in PID_autotune(float, int, int)() ** CID 59639: Out-of-bounds write (OVERRUN) /Marlin_main.cpp: 2278 in process_commands()() ** CID 59640: Out-of-bounds read (OVERRUN) /Marlin_main.cpp: 1802 in process_commands()() ** CID 59641: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 51 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() ** CID 59642: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 45 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() ** CID 59643: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 32 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() ________________________________________________________________________________________________________ *** CID 59629: Unchecked return value (CHECKED_RETURN) /Marlin_main.cpp: 2154 in process_commands()() 2148 } 2149 #endif 2150 } 2151 } 2152 break; 2153 case 85: // M85 CID 59629: Unchecked return value (CHECKED_RETURN) Calling "code_seen" without checking return value (as is done elsewhere 66 out of 67 times). 2154 code_seen('S'); 2155 max_inactive_time = code_value() * 1000; 2156 break; 2157 case 92: // M92 2158 for(int8_t i=0; i < NUM_AXIS; i++) 2159 { ________________________________________________________________________________________________________ *** CID 59630: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) /Applications/Arduino.app/Contents/Resources/Java/hardware/arduino/cores/arduino/Tone.cpp: 319 in tone(unsigned char, unsigned int, unsigned long)() 313 else 314 { 315 // two choices for the 16 bit timers: ck/1 or ck/64 316 ocr = F_CPU / frequency / 2 - 1; 317 318 prescalarbits = 0b001; CID 59630: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) "ocr > 65535U" is always false regardless of the values of its operands. This occurs as the logical operand of if. 319 if (ocr > 0xffff) 320 { 321 ocr = F_CPU / frequency / 2 / 64 - 1; 322 prescalarbits = 0b011; 323 } 324 ________________________________________________________________________________________________________ *** CID 59631: Missing break in switch (MISSING_BREAK) /Marlin_main.cpp: 1187 in process_commands()() 1181 case 2: // G2 - CW ARC 1182 if(Stopped == false) { 1183 get_arc_coordinates(); 1184 prepare_arc_move(true); 1185 return; 1186 } CID 59631: Missing break in switch (MISSING_BREAK) The above case falls through to this one. 1187 case 3: // G3 - CCW ARC 1188 if(Stopped == false) { 1189 get_arc_coordinates(); 1190 prepare_arc_move(false); 1191 return; 1192 } ________________________________________________________________________________________________________ *** CID 59632: Missing break in switch (MISSING_BREAK) /Marlin_main.cpp: 1193 in process_commands()() 1187 case 3: // G3 - CCW ARC 1188 if(Stopped == false) { 1189 get_arc_coordinates(); 1190 prepare_arc_move(false); 1191 return; 1192 } CID 59632: Missing break in switch (MISSING_BREAK) The above case falls through to this one. 1193 case 4: // G4 dwell 1194 LCD_MESSAGEPGM(MSG_DWELL); 1195 codenum = 0; 1196 if(code_seen('P')) codenum = code_value(); // milliseconds to wait 1197 if(code_seen('S')) codenum = code_value() * 1000; // seconds to wait 1198 ________________________________________________________________________________________________________ *** CID 59633: Out-of-bounds write (OVERRUN) /temperature.cpp: 914 in disable_heater()() 908 WRITE(HEATER_0_PIN,LOW); 909 #endif 910 #endif 911 912 #if defined(TEMP_1_PIN) && TEMP_1_PIN > -1 913 target_temperature[1]=0; CID 59633: Out-of-bounds write (OVERRUN) Overrunning array "soft_pwm" of 1 bytes at byte offset 1 using index "1". 914 soft_pwm[1]=0; 915 #if defined(HEATER_1_PIN) && HEATER_1_PIN > -1 916 WRITE(HEATER_1_PIN,LOW); 917 #endif 918 #endif 919 ________________________________________________________________________________________________________ *** CID 59634: Out-of-bounds write (OVERRUN) /temperature.cpp: 913 in disable_heater()() 907 #if defined(HEATER_0_PIN) && HEATER_0_PIN > -1 908 WRITE(HEATER_0_PIN,LOW); 909 #endif 910 #endif 911 912 #if defined(TEMP_1_PIN) && TEMP_1_PIN > -1 CID 59634: Out-of-bounds write (OVERRUN) Overrunning array "target_temperature" of 1 2-byte elements at element index 1 (byte offset 2) using index "1". 913 target_temperature[1]=0; 914 soft_pwm[1]=0; 915 #if defined(HEATER_1_PIN) && HEATER_1_PIN > -1 916 WRITE(HEATER_1_PIN,LOW); 917 #endif 918 #endif ________________________________________________________________________________________________________ *** CID 59635: Out-of-bounds read (OVERRUN) /temperature.cpp: 626 in analog2temp(int, unsigned char)() 620 if(heater_ttbl_map[e] != NULL) 621 { 622 float celsius = 0; 623 uint8_t i; 624 short (*tt)[][2] = (short (*)[][2])(heater_ttbl_map[e]); 625 CID 59635: Out-of-bounds read (OVERRUN) Overrunning array "heater_ttbllen_map" of 1 bytes at byte offset 1 using index "e" (which evaluates to 1). 626 for (i=1; i raw) 629 { 630 celsius = PGM_RD_W((*tt)[i-1][1]) + 631 (raw - PGM_RD_W((*tt)[i-1][0])) * ________________________________________________________________________________________________________ *** CID 59636: Out-of-bounds read (OVERRUN) /temperature.cpp: 620 in analog2temp(int, unsigned char)() 614 if (e == 0) 615 { 616 return 0.25 * raw; 617 } 618 #endif 619 CID 59636: Out-of-bounds read (OVERRUN) Overrunning array "heater_ttbl_map" of 1 2-byte elements at element index 1 (byte offset 2) using index "e" (which evaluates to 1). 620 if(heater_ttbl_map[e] != NULL) 621 { 622 float celsius = 0; 623 uint8_t i; 624 short (*tt)[][2] = (short (*)[][2])(heater_ttbl_map[e]); 625 ________________________________________________________________________________________________________ *** CID 59637: Out-of-bounds write (OVERRUN) /temperature.cpp: 202 in PID_autotune(float, int, int)() 196 { 197 soft_pwm_bed = (MAX_BED_POWER)/2; 198 bias = d = (MAX_BED_POWER)/2; 199 } 200 else 201 { CID 59637: Out-of-bounds write (OVERRUN) Overrunning array "soft_pwm" of 1 bytes at byte offset 1 using index "extruder" (which evaluates to 1). 202 soft_pwm[extruder] = (PID_MAX)/2; 203 bias = d = (PID_MAX)/2; 204 } 205 206 207 ________________________________________________________________________________________________________ *** CID 59638: Out-of-bounds read (OVERRUN) /temperature.cpp: 214 in PID_autotune(float, int, int)() 208 209 for(;;) { 210 211 if(temp_meas_ready == true) { // temp sample ready 212 updateTemperaturesFromRawValues(); 213 CID 59638: Out-of-bounds read (OVERRUN) Overrunning array "current_temperature" of 1 4-byte elements at element index 1 (byte offset 4) using index "extruder" (which evaluates to 1). 214 input = (extruder<0)?current_temperature_bed:current_temperature[extruder]; 215 216 max=max(max,input); 217 min=min(min,input); 218 if(heating == true && input > temp) { 219 if(millis() - t2 > 5000) { ________________________________________________________________________________________________________ *** CID 59639: Out-of-bounds write (OVERRUN) /Marlin_main.cpp: 2278 in process_commands()() 2272 tmp_extruder = code_value(); 2273 if(tmp_extruder >= EXTRUDERS) { 2274 SERIAL_ECHO_START; 2275 SERIAL_ECHO(MSG_M200_INVALID_EXTRUDER); 2276 } 2277 } CID 59639: Out-of-bounds write (OVERRUN) Overrunning array "volumetric_multiplier" of 1 4-byte elements at element index 1 (byte offset 4) using index "tmp_extruder" (which evaluates to 1). 2278 volumetric_multiplier[tmp_extruder] = 1 / area; 2279 } 2280 break; 2281 case 201: // M201 2282 for(int8_t i=0; i < NUM_AXIS; i++) 2283 { ________________________________________________________________________________________________________ *** CID 59640: Out-of-bounds read (OVERRUN) /Marlin_main.cpp: 1802 in process_commands()() 1796 int pin_status = code_value(); 1797 int pin_number = LED_PIN; 1798 if (code_seen('P') && pin_status >= 0 && pin_status <= 255) 1799 pin_number = code_value(); 1800 for(int8_t i = 0; i < (int8_t)sizeof(sensitive_pins); i++) 1801 { CID 59640: Out-of-bounds read (OVERRUN) Overrunning array "sensitive_pins" of 28 2-byte elements at element index 55 (byte offset 110) using index "i" (which evaluates to 55). 1802 if (sensitive_pins[i] == pin_number) 1803 { 1804 pin_number = -1; 1805 break; 1806 } 1807 } ________________________________________________________________________________________________________ *** CID 59641: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 51 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() 45 } 46 47 LiquidCrystal::LiquidCrystal(uint8_t rs, uint8_t enable, 48 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3) 49 { 50 init(1, rs, 255, enable, d0, d1, d2, d3, 0, 0, 0, 0); CID 59641: Uninitialized scalar field (UNINIT_CTOR) Non-static class member "_initialized" is not initialized in this constructor nor in any functions that it calls. 51 } 52 53 void LiquidCrystal::init(uint8_t fourbitmode, uint8_t rs, uint8_t rw, uint8_t enable, 54 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3, 55 uint8_t d4, uint8_t d5, uint8_t d6, uint8_t d7) 56 { ________________________________________________________________________________________________________ *** CID 59642: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 45 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() 39 } 40 41 LiquidCrystal::LiquidCrystal(uint8_t rs, uint8_t rw, uint8_t enable, 42 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3) 43 { 44 init(1, rs, rw, enable, d0, d1, d2, d3, 0, 0, 0, 0); CID 59642: Uninitialized scalar field (UNINIT_CTOR) Non-static class member "_initialized" is not initialized in this constructor nor in any functions that it calls. 45 } 46 47 LiquidCrystal::LiquidCrystal(uint8_t rs, uint8_t enable, 48 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3) 49 { 50 init(1, rs, 255, enable, d0, d1, d2, d3, 0, 0, 0, 0); ________________________________________________________________________________________________________ *** CID 59643: Uninitialized scalar field (UNINIT_CTOR) /Applications/Arduino.app/Contents/Resources/Java/libraries/LiquidCrystal/LiquidCrystal.cpp: 32 in LiquidCrystal::LiquidCrystal(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char)() 26 27 LiquidCrystal::LiquidCrystal(uint8_t rs, uint8_t rw, uint8_t enable, 28 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3, 29 uint8_t d4, uint8_t d5, uint8_t d6, uint8_t d7) 30 { 31 init(0, rs, rw, enable, d0, d1, d2, d3, d4, d5, d6, d7); CID 59643: Uninitialized scalar field (UNINIT_CTOR) Non-static class member "_initialized" is not initialized in this constructor nor in any functions that it calls. 32 } 33 34 LiquidCrystal::LiquidCrystal(uint8_t rs, uint8_t enable, 35 uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3, 36 uint8_t d4, uint8_t d5, uint8_t d6, uint8_t d7) 37 { ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, http://scan.coverity.com/projects/2224?tab=overview --- Marlin/Marlin_main.cpp | 12 ++++++++---- Marlin/temperature.cpp | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Marlin/Marlin_main.cpp b/Marlin/Marlin_main.cpp index 420bd00abb..ebc36bc1af 100644 --- a/Marlin/Marlin_main.cpp +++ b/Marlin/Marlin_main.cpp @@ -1177,19 +1177,21 @@ void process_commands() //ClearToSend(); return; } - //break; + break; case 2: // G2 - CW ARC if(Stopped == false) { get_arc_coordinates(); prepare_arc_move(true); return; } + break; case 3: // G3 - CCW ARC if(Stopped == false) { get_arc_coordinates(); prepare_arc_move(false); return; } + break; case 4: // G4 dwell LCD_MESSAGEPGM(MSG_DWELL); codenum = 0; @@ -1797,7 +1799,7 @@ void process_commands() int pin_number = LED_PIN; if (code_seen('P') && pin_status >= 0 && pin_status <= 255) pin_number = code_value(); - for(int8_t i = 0; i < (int8_t)sizeof(sensitive_pins); i++) + for(int8_t i = 0; i < (int8_t)(sizeof(sensitive_pins)/sizeof(int)); i++) { if (sensitive_pins[i] == pin_number) { @@ -2151,8 +2153,9 @@ void process_commands() } break; case 85: // M85 - code_seen('S'); - max_inactive_time = code_value() * 1000; + if(code_seen('S')) { + max_inactive_time = code_value() * 1000; + } break; case 92: // M92 for(int8_t i=0; i < NUM_AXIS; i++) @@ -2273,6 +2276,7 @@ void process_commands() if(tmp_extruder >= EXTRUDERS) { SERIAL_ECHO_START; SERIAL_ECHO(MSG_M200_INVALID_EXTRUDER); + break; } } volumetric_multiplier[tmp_extruder] = 1 / area; diff --git a/Marlin/temperature.cpp b/Marlin/temperature.cpp index 737d075754..a199f4e742 100644 --- a/Marlin/temperature.cpp +++ b/Marlin/temperature.cpp @@ -179,7 +179,7 @@ void PID_autotune(float temp, int extruder, int ncycles) float Kp, Ki, Kd; float max = 0, min = 10000; - if ((extruder > EXTRUDERS) + if ((extruder >= EXTRUDERS) #if (TEMP_BED_PIN <= -1) ||(extruder < 0) #endif @@ -909,7 +909,7 @@ void disable_heater() #endif #endif - #if defined(TEMP_1_PIN) && TEMP_1_PIN > -1 + #if defined(TEMP_1_PIN) && TEMP_1_PIN > -1 && EXTRUDERS > 1 target_temperature[1]=0; soft_pwm[1]=0; #if defined(HEATER_1_PIN) && HEATER_1_PIN > -1 @@ -917,7 +917,7 @@ void disable_heater() #endif #endif - #if defined(TEMP_2_PIN) && TEMP_2_PIN > -1 + #if defined(TEMP_2_PIN) && TEMP_2_PIN > -1 && EXTRUDERS > 2 target_temperature[2]=0; soft_pwm[2]=0; #if defined(HEATER_2_PIN) && HEATER_2_PIN > -1