Skip to content

Commit

Permalink
[dsme] Work around snprintf() warnings. Contributes to JB#46923
Browse files Browse the repository at this point in the history 
Attmpting to build dsme with gcc 8.x toolchain result in multiple
snprintf() related warnings.

Go through code that is triggering warnings and depending on situation:

- Adjust buffer sizes when applicable
- Check for failure/truncated and take sensible action
- Refactor the offending logic, or
- Silence warnings by using dummy return value check

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
  • Loading branch information
@spiiroin
spiiroin committed Sep 6, 2019
1 parent e629f17 commit bd56ad5
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 111 deletions.
2 changes: 1 addition & 1 deletion Makefile.custom
View file
Expand Up @@ -330,6 +330,7 @@ CLEAN_SOURCES += dsme/utility.c
CLEAN_SOURCES += modules/abootsettings.c
CLEAN_SOURCES += modules/alarmtracker.c
CLEAN_SOURCES += modules/batterytracker.c
CLEAN_SOURCES += modules/bootreasonlogger.c
CLEAN_SOURCES += modules/dbusautoconnector.c
CLEAN_SOURCES += modules/dbusproxy.c
CLEAN_SOURCES += modules/diskmonitor.c
Expand Down Expand Up @@ -375,7 +376,6 @@ DIRTY_SOURCES += dsme/mainloop.c
DIRTY_SOURCES += dsme/modulebase.c
DIRTY_SOURCES += dsme/oom.c
DIRTY_SOURCES += getbootstate/getbootstate.c
DIRTY_SOURCES += modules/bootreasonlogger.c
DIRTY_SOURCES += modules/emergencycalltracker.c
DIRTY_SOURCES += modules/iphb.c
DIRTY_SOURCES += modules/malf.c
Expand Down
104 changes: 53 additions & 51 deletions modules/bootreasonlogger.c
View file
Expand Up @@ -76,8 +76,6 @@ static const char *possible_pwrup_strings[] = {
0
};

static char pwrup_reason[80];

static const struct {
int value;
const char* name;
Expand Down Expand Up @@ -110,7 +108,7 @@ static bool sw_update_running(void)
static bool system_still_booting(void)
{
/* Once system boot is over, init-done flag is set */
/* If file is not there, we are still booting */
/* If file is not there, we are still booting */
return (access("/run/systemd/boot-status/init-done", F_OK) != 0);
}

Expand All @@ -132,7 +130,8 @@ static const char * get_timestamp(void)
((timeinfo = localtime(&raw_time)) != NULL) &&
(strftime(date_time, sizeof(date_time), "%Y%m%d_%H%M%S", timeinfo) > 0)) {
timestamp = date_time;
} else
}
else
dsme_log(LOG_ERR, PFIX"failed to get timestamp");

return timestamp;
Expand All @@ -157,74 +156,77 @@ static void write_log(const char *state, const char *reason)
}
}


static int get_cmd_line_value(char* get_value, int max_len, const char* key)
{
FILE* cmdline_file;
char cmdline[MAX_CMDLINE_LEN];
int ret = -1;
int keylen;
char* key_and_value;
char* value;

cmdline_file = fopen("/proc/cmdline", "r");
if(!cmdline_file) {
dsme_log(LOG_ERR, PFIX"Could not open /proc/cmdline");
return -1;
static const char path[] = "/proc/cmdline";

int ret = -1;
FILE *file = 0;

if( !(file = fopen(path, "r")) ) {
dsme_log(LOG_ERR, PFIX"Could not open %s: %m", path);
goto EXIT;
}

if (fgets(cmdline, MAX_CMDLINE_LEN, cmdline_file)) {
key_and_value = strtok(cmdline, " ");
keylen = strlen(key);
while (key_and_value != NULL) {
if(!strncmp(key_and_value, key, keylen)) {
value = strtok(key_and_value, "=");
value = strtok(NULL, "=");
if (value) {
snprintf(get_value, max_len, "%s", value);
ret = strlen(get_value);
}
break;
char cmdline[MAX_CMDLINE_LEN];
if( !fgets(cmdline, sizeof cmdline, file) ) {
dsme_log(LOG_ERR, PFIX"Could not read %s: %m", path);
goto EXIT;
}

for( char *parse = cmdline; parse; ) {
char *value = strsep(&parse, " \n");
char *entry = strsep(&value, "=");
if( !strcmp(entry, key) ) {
if( snprintf(get_value, max_len, "%s", value ?: "") >= max_len ) {
// dontcare - if it does not fit, we want it truncated
}
key_and_value = strtok(NULL, " ");
ret = strlen(get_value);
break;
}
}
fclose(cmdline_file);

EXIT:
if( file )
fclose(file);
return ret;
}

static char * get_powerup_reason_str(void)
{
char *env;
const char *search_key;
int i = 0;
char cmdvalue[80];

/* set default */
snprintf(pwrup_reason, sizeof(pwrup_reason), "Reason Unknown");

/* Powerup reason is either in enviroment or in kernel commandline
* we will look both and use first match
*/

while ((search_key = possible_pwrup_strings[i])) {
if ((env = getenv(search_key))) {
snprintf(pwrup_reason, sizeof(pwrup_reason),"%s=%s", search_key, env);
break;
} else if ((get_cmd_line_value(cmdvalue, sizeof(cmdvalue), search_key)) > 0) {
snprintf(pwrup_reason, sizeof(pwrup_reason),"%s=%s", search_key, cmdvalue);
break;
char *pwrup_reason = 0;
const char *search_key;
for( int i = 0; (search_key = possible_pwrup_strings[i]); ++i ) {
const char *env;
if( (env = getenv(search_key)) ) {
if( asprintf(&pwrup_reason, "%s=%s", search_key, env) < 0 )
pwrup_reason = 0;
else
break;
}

char cmdvalue[80];
if( (get_cmd_line_value(cmdvalue, sizeof cmdvalue, search_key) ) > 0) {
if( asprintf(&pwrup_reason, "%s=%s", search_key, cmdvalue) < 0 )
pwrup_reason = 0;
else
break;
}
i++;
}
return pwrup_reason;
}


static void log_startup(void)
{
if (system_still_booting())
write_log("Startup: ", get_powerup_reason_str());
if (system_still_booting()) {
char *pwrup_reason = get_powerup_reason_str();
write_log("Startup: ", pwrup_reason ?: "Reason Unknown");
free(pwrup_reason);
}
else {
/* System has already booted. We are here because
* dsme daemon has been restarted
Expand Down Expand Up @@ -263,7 +265,8 @@ DSME_HANDLER(DSM_MSGTYPE_SET_THERMAL_STATUS, conn, msg)
if (msg->status == DSM_THERMAL_STATUS_OVERHEATED) {
temp_status = "overheated";
overheated = true;
} else if (msg->status == DSM_THERMAL_STATUS_LOWTEMP)
}
else if (msg->status == DSM_THERMAL_STATUS_LOWTEMP)
temp_status = "low warning";
else
temp_status = "normal";
Expand Down Expand Up @@ -326,7 +329,6 @@ module_fn_info_t message_handlers[] = {
{0}
};


void module_init(module_t* handle)
{
dsme_log(LOG_DEBUG, "bootreasonlogger.so loaded");
Expand Down
62 changes: 40 additions & 22 deletions modules/iphb.c
View file
Expand Up @@ -102,6 +102,10 @@
/** Where to persistently store alarm state data received from timed */
#define XTIMED_STATE_FILE "/var/lib/dsme/timed_state"


/** Maximum expected lenght of human readable time stamp */
#define TIMESTAMP_MAX 64

/* ------------------------------------------------------------------------- *
* Custom types
* ------------------------------------------------------------------------- */
Expand Down Expand Up @@ -304,7 +308,9 @@ static void log_time_t(int lev, const char *title, time_t t, time_t now)
goto cleanup;
}

snprintf(left, sizeof left, " (T%+ld)", (long)(now - t));
if( snprintf(left, sizeof left, " (T%+ld)", (long)(now - t)) < 0 ) {
// dontcare - diagnostic logging
}

memset(&tm, 0, sizeof tm);
gmtime_r(&t, &tm);
Expand Down Expand Up @@ -402,14 +408,16 @@ static void wakelock_lock(const char *name, int ms)
dsme_log(LOG_DEBUG, PFIX"LOCK: %s %d", name, ms);
if( wakelock_supported() ) {
char tmp[256];
int rc = -1;
if( ms < 0 ) {
snprintf(tmp, sizeof tmp, "%s\n", name);
rc = snprintf(tmp, sizeof tmp, "%s\n", name);
}
else {
long long ns = ms * 1000000LL;
snprintf(tmp, sizeof tmp, "%s %lld\n", name, ns);
rc = snprintf(tmp, sizeof tmp, "%s %lld\n", name, ns);
}
wakelock_write(lock_path, tmp, -1);
if( rc > 0 && rc < sizeof tmp )
wakelock_write(lock_path, tmp, -1);
}
}

Expand All @@ -424,9 +432,10 @@ static void wakelock_unlock(const char *name)
dsme_log(LOG_DEBUG, PFIX"UNLK: %s", name);
if( wakelock_supported() ) {
char tmp[256];
snprintf(tmp, sizeof tmp, "%s\n", name);
int rc = snprintf(tmp, sizeof tmp, "%s\n", name);
/* assume EINVAL == the wakelock did not exist */
wakelock_write(unlock_path, tmp, EINVAL);
if( rc > 0 && rc < sizeof tmp )
wakelock_write(unlock_path, tmp, EINVAL);
}
}

Expand Down Expand Up @@ -789,7 +798,7 @@ static void linux_alarm_set(time_t delay)
if( linux_alarm_timerfd == -1 )
goto cleanup;

char tmp[32];
char tmp[TIMESTAMP_MAX];

struct timespec now = { .tv_sec = 0, .tv_nsec = 0 };

Expand Down Expand Up @@ -884,7 +893,7 @@ static void android_alarm_set(time_t delay)
if( android_alarm_fd == -1 )
goto cleanup;

char tmp[32];
char tmp[TIMESTAMP_MAX];

int get_time = ANDROID_ALARM_GET_TIME(ANDROID_ALARM_RTC);
if( ioctl(android_alarm_fd, get_time, &now) != 0 ) {
Expand Down Expand Up @@ -1018,7 +1027,7 @@ static void deltatime_set(time_t delta)
char tmp[32];
int len = snprintf(tmp, sizeof tmp, "%ld\n", (long)delta);

if( len > 0 && write(fd, tmp, len) == -1 ) {
if( len > 0 && len < sizeof tmp && write(fd, tmp, len) == -1 ) {
dsme_log(LOG_ERR, PFIX"%s: %s: %m", DELTATIME_CACHE_FILE, "write");
goto cleanup;
}
Expand Down Expand Up @@ -1069,14 +1078,18 @@ static void deltatime_update(void)
*/
static char *tm_repr(const struct tm *tm, char *buff, size_t size)
{
snprintf(buff, size, "%04d-%02d-%02d %02d:%02d:%02d %s",
tm->tm_year + 1900,
tm->tm_mon + 1,
tm->tm_mday + 0,
tm->tm_hour,
tm->tm_min,
tm->tm_sec,
tm->tm_zone ?: "???");
int rc = snprintf(buff, size, "%04d-%02d-%02d %02d:%02d:%02d %s",
tm->tm_year + 1900,
tm->tm_mon + 1,
tm->tm_mday + 0,
tm->tm_hour,
tm->tm_min,
tm->tm_sec,
tm->tm_zone ?: "???");
if( rc < 0 ) {
// dontcare - diagnostic logging
}

return buff;
}

Expand Down Expand Up @@ -1403,7 +1416,7 @@ static bool rtc_set_alarm_after(time_t delay)
time_t alm = sys + delay;

struct tm tm;
char tmp[32];
char tmp[TIMESTAMP_MAX];

dsme_log(LOG_INFO, PFIX"wakeup delay %d", (int)delay);
dsme_log(LOG_INFO, PFIX"system : %s", t_repr(sys, tmp, sizeof tmp));
Expand Down Expand Up @@ -2298,10 +2311,15 @@ static char *time_minus(const struct timeval *tv, char *buff, size_t size)
add(n);
if( d )
{
snprintf(tmp, sizeof tmp, "%ld days, ", d);
if( snprintf(tmp, sizeof tmp, "%ld days, ", d) < 0 ) {
// dontcare - diagnostic logging
}
add(tmp);
}
snprintf(tmp, sizeof tmp, "%02ld:%02ld:%02ld.%03ld", h, m, s, ms);
if( snprintf(tmp, sizeof tmp, "%02ld:%02ld:%02ld.%03ld",
h, m, s, ms) < 0 ) {
// dontcare - diagnostic logging
}
add(tmp);
return *pos = 0, buff;
}
Expand Down Expand Up @@ -3140,7 +3158,7 @@ static time_t mintime_fetch(void)
time_t t_release = get_mtime(IMAGE_TIME_STAMP_FILE);
time_t t_saved = get_mtime(SAVED_TIME_FILE);
time_t t_system = time(0);
char tmp[32];
char tmp[TIMESTAMP_MAX];

dsme_log(LOG_INFO, PFIX"builtin %s", t_repr(t_builtin, tmp, sizeof tmp));
dsme_log(LOG_INFO, PFIX"release %s", t_repr(t_release, tmp, sizeof tmp));
Expand Down Expand Up @@ -3171,7 +3189,7 @@ static void mintime_store(void)

static void systemtime_init(void)
{
char tmp[32];
char tmp[TIMESTAMP_MAX];
struct tm tm;

/* Get current state */
Expand Down
9 changes: 7 additions & 2 deletions modules/pwrkeymonitor.c
View file
Expand Up @@ -510,7 +510,7 @@ start_pwrkey_monitor(void)
DIR *dir = 0;
int cnt = 0;

char path[256];
char path[PATH_MAX];
struct dirent *de;

if( !(dir = opendir(base)) )
Expand All @@ -526,7 +526,12 @@ start_pwrkey_monitor(void)
continue;
}

snprintf(path, sizeof path, "%s/%s", base, de->d_name);
int rc = snprintf(path, sizeof path, "%s/%s", base, de->d_name);
if( rc < 0 || rc >= sizeof path )
{
/* Skip on error / truncate */
continue;
}

if( probe_evdev_device(path) )
{
Expand Down

0 comments on commit bd56ad5

Please sign in to comment.