Skip to content

Commit

Permalink
Explicitly check sscanf(3) and fscanf(3) return values
Browse files Browse the repository at this point in the history
Compare the return value of sscanf(3) and fscanf(3) explicitly against
the expected number of parsed items and avoid implicit boolean
conversion.  Such an implicit conversion would treat EOF (-1) the same
as at least one item parsed successfully.

Reported by CodeQL.
  • Loading branch information
cgzones authored and BenBE committed Jan 25, 2024
1 parent 207db2e commit 94c7822
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 16 deletions.
5 changes: 2 additions & 3 deletions Header.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,9 @@ static void Header_addMeterByName(Header* this, const char* name, MeterModeId mo
unsigned int param = 0;
size_t nameLen;
if (paren) {
int ok = sscanf(paren, "(%10u)", &param); // CPUMeter
if (!ok) {
if (sscanf(paren, "(%10u)", &param) != 1) { // not CPUMeter
char dynamic[32] = {0};
if (sscanf(paren, "(%30s)", dynamic)) { // DynamicMeter
if (sscanf(paren, "(%30s)", dynamic) == 1) { // DynamicMeter
char* end;
if ((end = strrchr(dynamic, ')')) == NULL)
return; // htoprc parse failure
Expand Down
2 changes: 1 addition & 1 deletion Settings.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ static int toFieldIndex(Hashtable* columns, const char* str) {
} else {
// Dynamically-defined columns are always stored by-name.
char dynamic[32] = {0};
if (sscanf(str, "Dynamic(%30s)", dynamic)) {
if (sscanf(str, "Dynamic(%30s)", dynamic) == 1) {
char* end;
if ((end = strrchr(dynamic, ')')) != NULL) {
bool success;
Expand Down
12 changes: 6 additions & 6 deletions linux/LinuxMachine.c
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ static void LinuxMachine_scanZramInfo(LinuxMachine* this) {
memory_t orig_data_size = 0;
memory_t compr_data_size = 0;

if (!fscanf(disksize_file, "%llu\n", &size) ||
!fscanf(mm_stat_file, " %llu %llu", &orig_data_size, &compr_data_size)) {
if (1 != fscanf(disksize_file, "%llu\n", &size) ||
2 != fscanf(mm_stat_file, " %llu %llu", &orig_data_size, &compr_data_size)) {
fclose(disksize_file);
fclose(mm_stat_file);
break;
Expand Down Expand Up @@ -342,10 +342,10 @@ static void LinuxMachine_scanZfsArcstats(LinuxMachine* this) {
sscanf(buffer + strlen(label), " %*2u %32llu", variable); \
break; \
} else (void) 0 /* Require a ";" after the macro use. */
#define tryReadFlag(label, variable, flag) \
if (String_startsWith(buffer, label)) { \
(flag) = sscanf(buffer + strlen(label), " %*2u %32llu", variable); \
break; \
#define tryReadFlag(label, variable, flag) \
if (String_startsWith(buffer, label)) { \
(flag) = (1 == sscanf(buffer + strlen(label), " %*2u %32llu", variable)); \
break; \
} else (void) 0 /* Require a ";" after the macro use. */

switch (buffer[0]) {
Expand Down
10 changes: 5 additions & 5 deletions linux/LinuxProcessTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -426,29 +426,29 @@ static bool LinuxProcessTable_readStatusFile(Process* process, openat_arg_t proc
} else if (String_startsWith(buffer, "voluntary_ctxt_switches:")) {
unsigned long vctxt;
int ok = sscanf(buffer, "voluntary_ctxt_switches:\t%lu", &vctxt);
if (ok >= 1) {
if (ok == 1) {
ctxt += vctxt;
}

} else if (String_startsWith(buffer, "nonvoluntary_ctxt_switches:")) {
unsigned long nvctxt;
int ok = sscanf(buffer, "nonvoluntary_ctxt_switches:\t%lu", &nvctxt);
if (ok >= 1) {
if (ok == 1) {
ctxt += nvctxt;
}

#ifdef HAVE_VSERVER
} else if (String_startsWith(buffer, "VxID:")) {
int vxid;
int ok = sscanf(buffer, "VxID:\t%32d", &vxid);
if (ok >= 1) {
if (ok == 1) {
lp->vxid = vxid;
}
#ifdef HAVE_ANCIENT_VSERVER
} else if (String_startsWith(buffer, "s_context:")) {
int vxid;
int ok = sscanf(buffer, "s_context:\t%32d", &vxid);
if (ok >= 1) {
if (ok == 1) {
lp->vxid = vxid;
}
#endif /* HAVE_ANCIENT_VSERVER */
Expand Down Expand Up @@ -938,7 +938,7 @@ static void LinuxProcessTable_readOomData(LinuxProcess* process, openat_arg_t pr
if (fgets(buffer, PROC_LINE_LENGTH, file)) {
unsigned int oom;
int ok = sscanf(buffer, "%u", &oom);
if (ok >= 1) {
if (ok == 1) {
process->oom = oom;
}
}
Expand Down
2 changes: 1 addition & 1 deletion linux/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ int Platform_getUptime(void) {
if (fd) {
int n = fscanf(fd, "%64lf", &uptime);
fclose(fd);
if (n <= 0) {
if (n != 1) {
return 0;
}
}
Expand Down

0 comments on commit 94c7822

Please sign in to comment.