starlingx/fault
Code Issues Proposed changes

replace strncpy by snprintf

Browse Source 
as for strncpy does not guarantee null terminated of dest string,
but snprintf does.

so change code from
    strncpy(buffer, source, sizeof buffer);
  to
    int n = snprintf(buffer, sizeof buffer, "%s", source);
    assert(n >= 0 && n < sizeof buffer);

test case:
  1. a simple test about difference between strncpy and snprintf
  2. use fmClientCli to create an alarm
     mock the init_tables() function so there is no need to
connect to the server.
     add code to dump SFmAlarmDataT to check datas return.
  3. one controller + one compute deploy (virtual)
     success deploy and create an instance.

Closes-Bug: 1794701

Change-Id: I04b4c5c85782ff571e2957e12c47f2f628d2b772
This commit is contained in:
SidneyAn 2018-11-03 00:39:28 +08:00
parent 105a2ea2c5
commit 816f45e94d
3 changed files with 37 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
fm-common/sources/fmAlarmUtils.cpp
View File

@ -36,14 +36,21 @@ static stoi_t cause_to_int;
static stoi_t bool_to_int;
static stoi_t err_to_int;
#define STRCP_TO(charb,str)\
if (((str).length()==0) || \
((str).length()==1 && ((str).c_str())[0]==' ')) { \
memset(charb,0,sizeof(charb)); \
} else { \
snprintf(charb, sizeof(charb)-1, "%s", str.c_str()); \
}
#define RETURN_FALSE return false
#define RETURN return
#define STRCP_TO(charb, str, return_action) \
do{ \
if (((str).length()==0) || \
((str).length()==1 && ((str).c_str())[0]==' ')) { \
memset(charb,0,sizeof(charb)); \
} else { \
int ret = snprintf(charb, sizeof(charb), "%s", str.c_str()); \
if (ret < 0) { \
FM_WARNING_LOG("STRCP_TO fail because of decode error."); \
return_action; \
} \
} \
}while(0)
void add_both_tables(int id, const char *str, itos_t &t1,stoi_t &t2 ) {
t1[id]=str;
@ -210,12 +217,12 @@ static void str_to_vector(const std::string &s, std::vector<std::string> &alarm)
static void fm_set_uuid(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->uuid);
else STRCP_TO(a->uuid,s);
else STRCP_TO(a->uuid,s, RETURN);
}
static void fm_tr_alarm_id(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->alarm_id);
else STRCP_TO(a->alarm_id,s);
else STRCP_TO(a->alarm_id,s, RETURN);
}
static void fm_alarm_state(SFmAlarmDataT *a, std::string &s, bool is_get) {
@ -225,12 +232,12 @@ static void fm_alarm_state(SFmAlarmDataT *a, std::string &s, bool is_get) {
static void fm_entity_id(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->entity_type_id);
else STRCP_TO(a->entity_type_id,s);
else STRCP_TO(a->entity_type_id,s, RETURN);
}
static void fm_instance_id(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->entity_instance_id);
else STRCP_TO(a->entity_instance_id,s);
else STRCP_TO(a->entity_instance_id,s, RETURN);
}
static void fm_timestamp(SFmAlarmDataT *a, std::string &s, bool is_get) {
@ -248,7 +255,7 @@ static void fm_alarm_severity(SFmAlarmDataT *a, std::string &s, bool is_get) {
static void fm_reason_text(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->reason_text);
else STRCP_TO(a->reason_text,s);
else STRCP_TO(a->reason_text,s, RETURN);
}
static void fm_alarm_type(SFmAlarmDataT *a, std::string &s, bool is_get) {
@ -263,7 +270,7 @@ static void fm_prop_cause(SFmAlarmDataT *a, std::string &s, bool is_get) {
static void fm_repair(SFmAlarmDataT *a, std::string &s, bool is_get) {
if(is_get) s = chkstr(a->proposed_repair_action);
else STRCP_TO(a->proposed_repair_action,s);
else STRCP_TO(a->proposed_repair_action,s, RETURN);
}
static void fm_service_affect(SFmAlarmDataT *a, std::string &s, bool is_get) {
@ -461,8 +468,8 @@ bool fm_alarm_filter_from_string(const std::string &str, AlarmFilter *filter) {
}
init_tables();
STRCP_TO(filter->alarm_id,s[0]);
STRCP_TO(filter->entity_instance_id,s[1]);
STRCP_TO(filter->alarm_id,s[0], RETURN_FALSE);
STRCP_TO(filter->entity_instance_id,s[1], RETURN_FALSE);
return true;
}

20
fm-common/sources/fmDbAlarm.cpp
View File

@ -44,7 +44,6 @@ static const char * field_map[] = {
FM_ALARM_COLUMN_MASKED //18
};
void add_both_tables(const char *str, int id, itos_t &t1,stoi_t &t2 ) {
t1[id]=str;
t2[str]=id;
@ -293,13 +292,10 @@ bool CFmDbAlarmOperation::get_alarms(CFmDBSession &sess,const char *id, fm_db_re
bool CFmDbAlarmOperation::get_alarms_by_id(CFmDBSession &sess,const char *id, fm_db_result_t & alarms) {
fm_alarm_id alm_id;
fm_alarm_id alm_id = {0};
char query[FM_MAX_SQL_STATEMENT_MAX];
std::string sql;
memset(alm_id, 0 , sizeof(alm_id));
strncpy(alm_id, id ,sizeof(alm_id)-1);
snprintf(alm_id, sizeof(alm_id), "%s", id);
snprintf(query, sizeof(query),"%s = '%s'", FM_ALARM_COLUMN_ALARM_ID, id);
fm_db_util_build_sql_query((const char*)FM_ALARM_TABLE_NAME, query, sql);
@ -338,8 +334,10 @@ bool CFmDbAlarmOperation::get_all_alarms(CFmDBSession &sess, SFmAlarmDataT **ala
CFmDbAlarm::convert_to(res[ix],p+ix);
std::string eid = (p+ix)->entity_instance_id;
eid = sname + "." + eid;
strncpy((p+ix)->entity_instance_id, eid.c_str(),
sizeof((p+ix)->entity_instance_id));
if (snprintf((p+ix)->entity_instance_id, sizeof((p+ix)->entity_instance_id),
"%s", eid.c_str()) < 0){
FM_WARNING_LOG("snprintf fail because of decode error.");
}
}
(*alarms) = p;
*len = found_num_alarms;
@ -454,8 +452,10 @@ bool CFmDbAlarmOperation::get_all_history_alarms(CFmDBSession &sess, SFmAlarmDat
CFmDbAlarm::convert_to(res[ix],p+ix);
std::string eid = (p+ix)->entity_instance_id;
eid = sname + "." + eid;
strncpy((p+ix)->entity_instance_id, eid.c_str(),
sizeof((p+ix)->entity_instance_id));
if (snprintf((p+ix)->entity_instance_id, sizeof((p+ix)->entity_instance_id),
"%s", eid.c_str()) < 0){
FM_WARNING_LOG("snprintf fail because of decode error.");
}
}
(*alarms) = p;
*len = found_num_alarms;

6
fm-common/sources/fmDbEventLog.cpp
View File

@ -309,8 +309,10 @@ bool CFmDbEventLogOperation::get_all_event_logs(CFmDBSession &sess, SFmAlarmData
CFmDbEventLog::convert_to(res[ix],p+ix);
std::string eid = (p+ix)->entity_instance_id;
eid = sname + "." + eid;
strncpy((p+ix)->entity_instance_id, eid.c_str(),
sizeof((p+ix)->entity_instance_id));
if(snprintf((p+ix)->entity_instance_id, sizeof((p+ix)->entity_instance_id), "%s", eid.c_str()) < 0) {
FM_WARNING_LOG("fail to snprintf eid %s to entity_instance_id for encoding error.", eid.c_str());
continue;
}
}
(*logs) = p;
*len = found_num_logs;