Skip to content

Commit decf311

Browse files
committed
Improve C standards compliance and code quality
This commit does the following: - Add void parameter lists for parameter-less functions - Fix pointer arithmetic - Initialize variables and buffers to prevent undefined behavior - Replace unsafe string operations with safe alternatives - Add early returns after error conditions - Make implicit comparisons explicit - Fix null pointer handling in memory allocation Change-Id: I2bf51bd2dd954bd9bcef2df6073e8a5509facea9
1 parent 09d0c7c commit decf311

File tree

7 files changed

+54
-48
lines changed

7 files changed

+54
-48
lines changed

console.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ static bool has_infile = false;
6565
static cmd_func_t quit_helpers[MAXQUIT];
6666
static int quit_helper_cnt = 0;
6767

68-
static void init_in();
68+
static void init_in(void);
6969

7070
static bool push_file(char *fname);
71-
static void pop_file();
71+
static void pop_file(void);
7272

7373
static bool interpret_cmda(int argc, char *argv[]);
7474

@@ -189,7 +189,7 @@ static bool force_quit(int argc, char *argv[])
189189
return ok;
190190
}
191191

192-
static void record_error()
192+
static void record_error(void)
193193
{
194194
err_cnt++;
195195
if (err_cnt >= err_limit) {
@@ -403,7 +403,7 @@ static bool do_time(int argc, char *argv[])
403403
}
404404

405405
static bool use_linenoise = true;
406-
static int web_fd;
406+
static int web_fd = -1;
407407

408408
static bool do_web(int argc, char *argv[])
409409
{
@@ -426,7 +426,7 @@ static bool do_web(int argc, char *argv[])
426426
}
427427

428428
/* Initialize interpreter */
429-
void init_cmd()
429+
void init_cmd(void)
430430
{
431431
cmd_list = NULL;
432432
param_list = NULL;
@@ -479,7 +479,7 @@ static bool push_file(char *fname)
479479
}
480480

481481
/* Pop a file buffer from stack */
482-
static void pop_file()
482+
static void pop_file(void)
483483
{
484484
if (buf_stack) {
485485
rio_t *rsave = buf_stack;
@@ -490,15 +490,15 @@ static void pop_file()
490490
}
491491

492492
/* Handling of input */
493-
static void init_in()
493+
static void init_in(void)
494494
{
495495
buf_stack = NULL;
496496
}
497497

498498
/* Read command from input file.
499499
* When hit EOF, close that file and return NULL
500500
*/
501-
static char *readline()
501+
static char *readline(void)
502502
{
503503
char c;
504504
char *lptr = linebuf;
@@ -551,7 +551,7 @@ static char *readline()
551551
return linebuf;
552552
}
553553

554-
static bool cmd_done()
554+
static bool cmd_done(void)
555555
{
556556
return !buf_stack || quit_flag;
557557
}
@@ -607,7 +607,7 @@ static int cmd_select(int nfds,
607607
return 0;
608608
}
609609

610-
bool finish_cmd()
610+
bool finish_cmd(void)
611611
{
612612
bool ok = true;
613613
if (!quit_flag)
@@ -618,7 +618,8 @@ bool finish_cmd()
618618

619619
static bool cmd_maybe(const char *target, const char *src)
620620
{
621-
for (int i = 0; i < strlen(src); i++) {
621+
size_t src_len = strlen(src);
622+
for (size_t i = 0; i < src_len; i++) {
622623
if (target[i] == '\0')
623624
return false;
624625
if (src[i] != target[i])
@@ -638,8 +639,8 @@ void completion(const char *buf, line_completions_t *lc)
638639
if (strlen(plist->name) > 120)
639640
continue;
640641

641-
strcat(str, "option ");
642-
strcat(str, plist->name);
642+
strncat(str, "option ", sizeof(str) - 1);
643+
strncat(str, plist->name, sizeof(str) - strlen(str) - 1);
643644
if (cmd_maybe(str, buf))
644645
line_add_completion(lc, str);
645646

console.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ typedef struct __param_element {
4141
} param_element_t;
4242

4343
/* Initialize interpreter */
44-
void init_cmd();
44+
void init_cmd(void);
4545

4646
/* Add a new command */
4747
void add_cmd(char *name, cmd_func_t operation, char *summary, char *parameter);
@@ -62,7 +62,7 @@ void set_echo(bool on);
6262
/* Complete command interpretation */
6363

6464
/* Return true if no errors occurred */
65-
bool finish_cmd();
65+
bool finish_cmd(void);
6666

6767
/* Run command loop. Non-null infile_name implies read commands from that file
6868
*/

harness.c

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ typedef enum {
6969
/* Internal functions */
7070

7171
/* Should this allocation fail? */
72-
static bool fail_allocation()
72+
static bool fail_allocation(void)
7373
{
7474
double weight = (double) random() / RAND_MAX;
7575
return (weight < 0.01 * fail_probability);
@@ -83,10 +83,11 @@ static block_element_t *find_header(void *p)
8383
if (!p) {
8484
report_event(MSG_ERROR, "Attempting to free null block");
8585
error_occurred = true;
86+
return NULL;
8687
}
8788

8889
block_element_t *b =
89-
(block_element_t *) ((size_t) p - sizeof(block_element_t));
90+
(block_element_t *) ((uintptr_t) p - sizeof(block_element_t));
9091
if (cautious_mode) {
9192
/* Make sure this is really an allocated block */
9293
block_element_t *ab = allocated;
@@ -119,7 +120,7 @@ static size_t *find_footer(block_element_t *b)
119120
{
120121
// cppcheck-suppress nullPointerRedundantCheck
121122
size_t *p =
122-
(size_t *) ((size_t) b + b->payload_size + sizeof(block_element_t));
123+
(size_t *) ((uintptr_t) b + b->payload_size + sizeof(block_element_t));
123124
return p;
124125
}
125126

@@ -150,18 +151,15 @@ static void *alloc(alloc_t alloc_type, size_t size)
150151
if (!new_block) {
151152
report_event(MSG_FATAL, "Couldn't allocate any more memory");
152153
error_occurred = true;
154+
return NULL;
153155
}
154156

155-
// cppcheck-suppress nullPointerRedundantCheck
156157
new_block->magic_header = MAGICHEADER;
157-
// cppcheck-suppress nullPointerRedundantCheck
158158
new_block->payload_size = size;
159159
*find_footer(new_block) = MAGICFOOTER;
160160
void *p = (void *) &new_block->payload;
161-
memset(p, !alloc_type * FILLCHAR, size);
162-
// cppcheck-suppress nullPointerRedundantCheck
161+
memset(p, (alloc_type == TEST_CALLOC) ? 0 : FILLCHAR, size);
163162
new_block->next = allocated;
164-
// cppcheck-suppress nullPointerRedundantCheck
165163
new_block->prev = NULL;
166164

167165
if (allocated)
@@ -260,7 +258,7 @@ char *test_strdup(const char *s)
260258
return memcpy(new, s, len);
261259
}
262260

263-
size_t allocation_check()
261+
size_t allocation_check(void)
264262
{
265263
return allocated_count;
266264
}
@@ -283,8 +281,8 @@ void set_noallocate_mode(bool noallocate)
283281
noallocate_mode = noallocate;
284282
}
285283

286-
/* Return whether any errors have occurred since last time set error limit */
287-
bool error_check()
284+
/* Return whether any errors have occurred since last time checked */
285+
bool error_check(void)
288286
{
289287
bool e = error_occurred;
290288
error_occurred = false;
@@ -320,7 +318,7 @@ bool exception_setup(bool limit_time)
320318
}
321319

322320
/* Call once past risky code */
323-
void exception_cancel()
321+
void exception_cancel(void)
324322
{
325323
if (time_limited) {
326324
alarm(0);

harness.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ char *test_strdup(const char *s);
1919
#ifdef INTERNAL
2020

2121
/* Report number of allocated blocks */
22-
size_t allocation_check();
22+
size_t allocation_check(void);
2323

2424
/* Probability of malloc failing, expressed as percent */
2525
extern int fail_probability;
@@ -37,15 +37,15 @@ void set_cautious_mode(bool cautious);
3737
void set_noallocate_mode(bool noallocate);
3838

3939
/* Return whether any errors have occurred since last time checked */
40-
bool error_check();
40+
bool error_check(void);
4141

4242
/* Prepare for a risky operation using setjmp.
4343
* Function returns true for initial return, false for error return
4444
*/
4545
bool exception_setup(bool limit_time);
4646

4747
/* Call once past risky code */
48-
void exception_cancel();
48+
void exception_cancel(void);
4949

5050
/* Use longjmp to return to most recent exception setup. Include error message
5151
*/

qtest.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ static bool do_merge(int argc, char *argv[])
913913
return ok && !error_check();
914914
}
915915

916-
static bool is_circular()
916+
static bool is_circular(void)
917917
{
918918
struct list_head *cur = current->q->next;
919919
struct list_head *fast = (cur) ? cur->next : NULL;
@@ -1056,7 +1056,7 @@ static bool do_next(int argc, char *argv[])
10561056
return q_show(0);
10571057
}
10581058

1059-
static void console_init()
1059+
static void console_init(void)
10601060
{
10611061
ADD_COMMAND(new, "Create new queue", "");
10621062
ADD_COMMAND(free, "Delete queue", "");
@@ -1125,7 +1125,7 @@ static void sigalrm_handler(int sig)
11251125
"code is too inefficient");
11261126
}
11271127

1128-
static void q_init()
1128+
static void q_init(void)
11291129
{
11301130
fail_count = 0;
11311131
INIT_LIST_HEAD(&chain.head);
@@ -1259,7 +1259,7 @@ bool commit_exists(const char *commit_hash)
12591259
char buffer[1024];
12601260
while (fgets(buffer, sizeof(buffer), stream)) {
12611261
/* Compare the first 40 characters of each line with commit_hash */
1262-
if (!strncmp(buffer, commit_hash, 40)) {
1262+
if (strncmp(buffer, commit_hash, 40) == 0) {
12631263
found = true;
12641264
break;
12651265
}
@@ -1294,19 +1294,20 @@ static bool check_commitlog(void)
12941294
}
12951295

12961296
#define GIT_HOOK ".git/hooks/"
1297-
static bool sanity_check()
1297+
static bool sanity_check(void)
12981298
{
12991299
struct stat buf;
13001300
/* Directory .git not found */
1301-
if (stat(".git", &buf)) {
1301+
if (stat(".git", &buf) != 0) {
13021302
fprintf(stderr,
13031303
"FATAL: You should run qtest in the directory containing valid "
13041304
"git workspace.\n");
13051305
return false;
13061306
}
13071307
/* Expected pre-commit and pre-push hooks not found */
1308-
if (stat(GIT_HOOK "commit-msg", &buf) ||
1309-
stat(GIT_HOOK "pre-commit", &buf) || stat(GIT_HOOK "pre-push", &buf)) {
1308+
if (stat(GIT_HOOK "commit-msg", &buf) != 0 ||
1309+
stat(GIT_HOOK "pre-commit", &buf) != 0 ||
1310+
stat(GIT_HOOK "pre-push", &buf) != 0) {
13101311
fprintf(stderr, "FATAL: Git hooks are not properly installed.\n");
13111312

13121313
/* Attempt to install Git hooks */
@@ -1323,7 +1324,7 @@ static bool sanity_check()
13231324
return false;
13241325
}
13251326
/* GitHub Actions checkouts do not include the complete git history. */
1326-
if (stat("/home/runner/work", &buf)) {
1327+
if (stat("/home/runner/work", &buf) != 0) {
13271328
#define COPYRIGHT_COMMIT_SHA1 "50c5ac53d31adf6baac4f8d3db6b3ce2215fee40"
13281329
if (!commit_exists(COPYRIGHT_COMMIT_SHA1)) {
13291330
fprintf(

report.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ static char fail_buf[1024] = "FATAL Error. Exiting\n";
3030
static volatile int ret = 0;
3131

3232
/* Default fatal function */
33-
static void default_fatal_fun()
33+
static void default_fatal_fun(void)
3434
{
3535
ret = write(STDOUT_FILENO, fail_buf, strlen(fail_buf) + 1);
3636
if (logfile)
3737
fputs(fail_buf, logfile);
3838
}
3939

4040
/* Optional function to call when fatal error encountered */
41-
static void (*fatal_fun)() = default_fatal_fun;
41+
static void (*fatal_fun)(void) = default_fatal_fun;
4242

4343
void set_verblevel(int level)
4444
{
@@ -102,7 +102,7 @@ void report(int level, char *fmt, ...)
102102
if (!verbfile)
103103
init_files(stdout, stdout);
104104

105-
char buffer[BUF_SIZE];
105+
char buffer[BUF_SIZE] = {0};
106106
if (level <= verblevel) {
107107
va_list ap;
108108
va_start(ap, fmt);
@@ -135,7 +135,7 @@ void report_noreturn(int level, char *fmt, ...)
135135
if (!verbfile)
136136
init_files(stdout, stdout);
137137

138-
char buffer[BUF_SIZE];
138+
char buffer[BUF_SIZE] = {0};
139139
if (level <= verblevel) {
140140
va_list ap;
141141
va_start(ap, fmt);
@@ -263,14 +263,17 @@ char *strsave_or_fail(const char *s, const char *fun_name)
263263
peak_bytes = MAX(peak_bytes, current_bytes);
264264
last_peak_bytes = MAX(last_peak_bytes, current_bytes);
265265

266+
// cppcheck-suppress returnDanglingLifetime
266267
return strncpy(ss, s, len + 1);
267268
}
268269

269270
/* Free block, as from malloc, realloc, or strsave */
270271
void free_block(void *b, size_t bytes)
271272
{
272-
if (!b)
273+
if (!b) {
273274
report_event(MSG_ERROR, "Attempting to free null block");
275+
return;
276+
}
274277
free(b);
275278

276279
free_cnt++;
@@ -281,8 +284,10 @@ void free_block(void *b, size_t bytes)
281284
/* Free array, as from calloc */
282285
void free_array(void *b, size_t cnt, size_t bytes)
283286
{
284-
if (!b)
287+
if (!b) {
285288
report_event(MSG_ERROR, "Attempting to free null block");
289+
return;
290+
}
286291
free(b);
287292

288293
free_cnt++;
@@ -293,8 +298,10 @@ void free_array(void *b, size_t cnt, size_t bytes)
293298
/* Free string saved by strsave_or_fail */
294299
void free_string(char *s)
295300
{
296-
if (!s)
301+
if (!s) {
297302
report_event(MSG_ERROR, "Attempting to free null block");
303+
return;
304+
}
298305
free_block((void *) s, strlen(s) + 1);
299306
}
300307

scripts/pre-commit.hook

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ cppcheck_suppressions() {
8585

8686
# Array for additional cppcheck options (non-suppressions)
8787
local -a other_flags=(
88-
"--inline-suppr harness.c --inline-suppr tools/fmtscan.c"
88+
"--inline-suppr"
8989
)
9090

9191
local out=""
@@ -376,7 +376,6 @@ if [ $RETURN -eq 0 ]; then
376376
printf "\n${GREEN}All checks passed!${NC}\n"
377377
else
378378
printf "\n${RED}Pre-commit checks failed. Please fix the issues above.${NC}\n"
379-
printf "${YELLOW}Tip: Use 'git commit --no-verify' to bypass these checks (not recommended).${NC}\n"
380379
fi
381380

382381
exit $RETURN

0 commit comments

Comments
 (0)