From 7929287f73fb34bcd710645cfa68bab726474be1 Mon Sep 17 00:00:00 2001 From: Julien Lecoeur Date: Tue, 30 May 2017 16:54:17 +0200 Subject: [PATCH] Fix -Werror=format-truncation on GCC 7 Fix formatting Check snprintf return for error AND overflow --- src/drivers/gps/gps.cpp | 3 +- src/drivers/pwm_out_sim/pwm_out_sim.cpp | 11 ++++-- src/drivers/vmount/input_mavlink.cpp | 3 +- src/modules/mavlink/mavlink_ftp.cpp | 35 +++++++++++------ src/modules/mavlink/mavlink_log_handler.cpp | 43 ++++++++++++++------- src/modules/navigator/follow_target.cpp | 3 +- 6 files changed, 64 insertions(+), 34 deletions(-) diff --git a/src/drivers/gps/gps.cpp b/src/drivers/gps/gps.cpp index ddd37700f7..a869181c97 100644 --- a/src/drivers/gps/gps.cpp +++ b/src/drivers/gps/gps.cpp @@ -684,7 +684,8 @@ GPS::task_main() switch (_mode) { case GPS_DRIVER_MODE_NONE: _mode = GPS_DRIVER_MODE_UBX; - /* FALLTHROUGH */ + + /* FALLTHROUGH */ case GPS_DRIVER_MODE_UBX: _helper = new GPSDriverUBX(_interface, &GPS::callback, this, &_report_gps_pos, _p_report_sat_info); diff --git a/src/drivers/pwm_out_sim/pwm_out_sim.cpp b/src/drivers/pwm_out_sim/pwm_out_sim.cpp index e72920fa41..b855f74e99 100644 --- a/src/drivers/pwm_out_sim/pwm_out_sim.cpp +++ b/src/drivers/pwm_out_sim/pwm_out_sim.cpp @@ -718,7 +718,8 @@ PWMSim::pwm_ioctl(device::file_t *filp, int cmd, unsigned long arg) ret = -EINVAL; break; } - /* FALLTHROUGH */ + + /* FALLTHROUGH */ case PWM_SERVO_SET(0): case PWM_SERVO_SET(1): @@ -740,7 +741,8 @@ PWMSim::pwm_ioctl(device::file_t *filp, int cmd, unsigned long arg) ret = -EINVAL; break; } - /* FALLTHROUGH */ + + /* FALLTHROUGH */ case PWM_SERVO_GET(3): case PWM_SERVO_GET(2): @@ -748,8 +750,9 @@ PWMSim::pwm_ioctl(device::file_t *filp, int cmd, unsigned long arg) ret = -EINVAL; break; } - /* FALLTHROUGH */ - + + /* FALLTHROUGH */ + case PWM_SERVO_GET(1): case PWM_SERVO_GET(0): { *(servo_position_t *)arg = 1500; diff --git a/src/drivers/vmount/input_mavlink.cpp b/src/drivers/vmount/input_mavlink.cpp index 5ebe7c36c7..cd44cca7d3 100644 --- a/src/drivers/vmount/input_mavlink.cpp +++ b/src/drivers/vmount/input_mavlink.cpp @@ -231,7 +231,8 @@ int InputMavlinkCmdMount::update_impl(unsigned int timeout_ms, ControlData **con switch ((int)vehicle_command.param7) { case vehicle_command_s::VEHICLE_MOUNT_MODE_RETRACT: _control_data.gimbal_shutter_retract = true; - /* FALLTHROUGH */ + + /* FALLTHROUGH */ case vehicle_command_s::VEHICLE_MOUNT_MODE_NEUTRAL: _control_data.type = ControlData::Type::Neutral; diff --git a/src/modules/mavlink/mavlink_ftp.cpp b/src/modules/mavlink/mavlink_ftp.cpp index 75986c2c8a..ae825c1019 100644 --- a/src/modules/mavlink/mavlink_ftp.cpp +++ b/src/modules/mavlink/mavlink_ftp.cpp @@ -355,20 +355,27 @@ MavlinkFTP::_workList(PayloadHeader *payload, bool list_hidden) switch (result->d_type) { #ifdef __PX4_NUTTX - case DTYPE_FILE: + case DTYPE_FILE: { #else - case DT_REG: -#endif - // For files we get the file size as well - direntType = kDirentFile; - snprintf(buf, sizeof(buf), "%s/%s", dirPath, result->d_name); - struct stat st; - if (stat(buf, &st) == 0) { - fileSize = st.st_size; + case DT_REG: { +#endif + // For files we get the file size as well + direntType = kDirentFile; + int ret = snprintf(buf, sizeof(buf), "%s/%s", dirPath, result->d_name); + bool buf_is_ok = ((ret > 0) && (ret < sizeof(buf))); + + if (buf_is_ok) { + struct stat st; + + if (stat(buf, &st) == 0) { + fileSize = st.st_size; + } + } + + break; } - break; #ifdef __PX4_NUTTX case DTYPE_DIRECTORY: @@ -397,7 +404,12 @@ MavlinkFTP::_workList(PayloadHeader *payload, bool list_hidden) } else if (direntType == kDirentFile) { // Files send filename and file length - snprintf(buf, sizeof(buf), "%s\t%d", result->d_name, fileSize); + int ret = snprintf(buf, sizeof(buf), "%s\t%d", result->d_name, fileSize); + bool buf_is_ok = ((ret > 0) && (ret < sizeof(buf))); + + if (!buf_is_ok) { + buf[sizeof(buf) - 1] = '\0'; + } } else { // Everything else just sends name @@ -995,4 +1007,3 @@ void MavlinkFTP::send(const hrt_abstime t) _reply(&ftp_msg); } while (more_data); } - diff --git a/src/modules/mavlink/mavlink_log_handler.cpp b/src/modules/mavlink/mavlink_log_handler.cpp index cfd62b5cf9..019c0dc541 100644 --- a/src/modules/mavlink/mavlink_log_handler.cpp +++ b/src/modules/mavlink/mavlink_log_handler.cpp @@ -475,10 +475,13 @@ LogListHelper::_init() if (result->d_type == PX4LOG_DIRECTORY) { time_t tt = 0; char log_path[128]; - snprintf(log_path, sizeof(log_path), "%s/%s", kLogRoot, result->d_name); + int ret = snprintf(log_path, sizeof(log_path), "%s/%s", kLogRoot, result->d_name); + bool path_is_ok = (ret > 0) && (ret < sizeof(log_path)); - if (_get_session_date(log_path, result->d_name, tt)) { - _scan_logs(f, log_path, tt); + if (path_is_ok) { + if (_get_session_date(log_path, result->d_name, tt)) { + _scan_logs(f, log_path, tt); + } } } } @@ -534,12 +537,15 @@ LogListHelper::_scan_logs(FILE *f, const char *dir, time_t &date) time_t ldate = date; uint32_t size = 0; char log_file_path[128]; - snprintf(log_file_path, sizeof(log_file_path), "%s/%s", dir, result->d_name); + int ret = snprintf(log_file_path, sizeof(log_file_path), "%s/%s", dir, result->d_name); + bool path_is_ok = (ret > 0) && (ret < sizeof(log_file_path)); - if (_get_log_time_size(log_file_path, result->d_name, ldate, size)) { - //-- Write result->out to list file - fprintf(f, "%u %u %s\n", (unsigned)ldate, (unsigned)size, log_file_path); - log_count++; + if (path_is_ok) { + if (_get_log_time_size(log_file_path, result->d_name, ldate, size)) { + //-- Write result->out to list file + fprintf(f, "%u %u %s\n", (unsigned)ldate, (unsigned)size, log_file_path); + log_count++; + } } } } @@ -601,20 +607,27 @@ LogListHelper::delete_all(const char *dir) if (result->d_type == PX4LOG_DIRECTORY && result->d_name[0] != '.') { char log_path[128]; - snprintf(log_path, sizeof(log_path), "%s/%s", dir, result->d_name); - LogListHelper::delete_all(log_path); + int ret = snprintf(log_path, sizeof(log_path), "%s/%s", dir, result->d_name); + bool path_is_ok = (ret > 0) && (ret < sizeof(log_path)); - if (rmdir(log_path)) { - PX4LOG_WARN("MavlinkLogHandler::delete_all Error removing %s\n", log_path); + if (path_is_ok) { + LogListHelper::delete_all(log_path); + + if (rmdir(log_path)) { + PX4LOG_WARN("MavlinkLogHandler::delete_all Error removing %s\n", log_path); + } } } if (result->d_type == PX4LOG_REGULAR_FILE) { char log_path[128]; - snprintf(log_path, sizeof(log_path), "%s/%s", dir, result->d_name); + int ret = snprintf(log_path, sizeof(log_path), "%s/%s", dir, result->d_name); + bool path_is_ok = (ret > 0) && (ret < sizeof(log_path)); - if (unlink(log_path)) { - PX4LOG_WARN("MavlinkLogHandler::delete_all Error deleting %s\n", log_path); + if (path_is_ok) { + if (unlink(log_path)) { + PX4LOG_WARN("MavlinkLogHandler::delete_all Error deleting %s\n", log_path); + } } } } diff --git a/src/modules/navigator/follow_target.cpp b/src/modules/navigator/follow_target.cpp index 61beef3b0b..27f0b7a86c 100644 --- a/src/modules/navigator/follow_target.cpp +++ b/src/modules/navigator/follow_target.cpp @@ -336,7 +336,8 @@ void FollowTarget::on_active() _follow_target_state = WAIT_FOR_TARGET_POSITION; } - /* FALLTHROUGH */ + + /* FALLTHROUGH */ case WAIT_FOR_TARGET_POSITION: {