Skip to content

Commit 92806ce

Browse files
author
Richard Young
committed
Fix pull request comments related to Linux kernel style.
Improved return logic and error handling of reopen_file Renamed log file argument processing function. Improved return error codes. Moved error handling responsibility outside of function. Add include header for error_t
1 parent ec631ef commit 92806ce

File tree

1 file changed

+65
-86
lines changed

1 file changed

+65
-86
lines changed

Diff for: candump.c

+65-86
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262

6363
#include <linux/can.h>
6464
#include <linux/can/raw.h>
65+
#include <errno.h>
6566

6667
#include "terminal.h"
6768
#include "lib.h"
@@ -111,18 +112,18 @@ const char anichar[MAXANI] = {'|', '/', '-', '\\'};
111112
const char extra_m_info[4][4] = {"- -", "B -", "- E", "B E"};
112113

113114
#define MAXLOGNAMESZ 100
114-
FILE *logfile = NULL;
115+
static FILE *logfile = NULL;
115116
static char log_filename[MAXLOGNAMESZ];
116117

117-
unsigned char silent = SILENT_INI;
118+
static unsigned char silent = SILENT_INI;
118119

119120
extern int optind, opterr, optopt;
120121

121122
static volatile int running = 1;
122-
static volatile int flag_reopen_file = 0;
123-
static volatile unsigned long sighup_count = 0;
124-
int is_auto_log_name = 0;
125-
int open_log_file(const char * filename);
123+
static volatile int flag_reopen_file;
124+
static volatile unsigned long sighup_count;
125+
126+
static int is_auto_log_name = 0;
126127

127128
void print_usage(char *prg)
128129
{
@@ -177,8 +178,7 @@ void sigterm(int signo)
177178

178179
void sighup(int signo)
179180
{
180-
if(signo == SIGHUP)
181-
{
181+
if (signo == SIGHUP && running) {
182182
flag_reopen_file = 1;
183183
sighup_count++;
184184
}
@@ -233,14 +233,13 @@ int idx2dindex(int ifidx, int socket) {
233233
return i;
234234
}
235235

236-
int sprint_auto_filename_format(char * buffer)
236+
static int sprint_auto_filename_format(char *buffer)
237237
{
238238
time_t currtime;
239239
struct tm now;
240240

241241
if (time(&currtime) == (time_t)-1) {
242242
perror("time");
243-
//running = 0;
244243
return 1;
245244
}
246245

@@ -259,14 +258,13 @@ int sprint_auto_filename_format(char * buffer)
259258
return 0;
260259
}
261260

262-
//opens file using global filehandle logfile
263-
int open_log_file(const char * fileName)
261+
/*opens file using global var logfile*/
262+
static int open_log_file(const char *file_name)
264263
{
265264
if (silent != SILENT_ON)
266265
fprintf(stderr, "Warning: Console output active while logging!\n");
267266

268-
const char * fopen_opts = (sighup_count > 0) ? "a" : "w";
269-
logfile = fopen(fileName, fopen_opts);
267+
logfile = fopen(file_name, "w");
270268

271269
if (!logfile) {
272270
perror("logfile");
@@ -276,55 +274,45 @@ int open_log_file(const char * fileName)
276274
return 0;
277275
}
278276

279-
//flush,close then reopen log
280-
int reopen_file(FILE * fileHandle)
277+
static int reopen_file(FILE *file_handle)
281278
{
282-
if(!fileHandle)
283-
return 1;
279+
const char *fopen_opts = (sighup_count > 0 && !is_auto_log_name) ? "a" : "w";
284280

285-
int errcode = 0;
281+
if (!file_handle)
282+
return 1;
286283

287-
if(is_auto_log_name == 1)
288-
{
289-
errcode = sprint_auto_filename_format(&log_filename[0]);
284+
if (is_auto_log_name == 1) {
285+
const int errcode = sprint_auto_filename_format(log_filename);
290286

291-
if(errcode != 0)
292-
{
293-
running = 0;
287+
if (errcode != 0) {
294288
return 1;
295289
}
296290
}
297291

298-
const char * fopen_opts = (sighup_count > 0) ? "a" : "w";
299-
logfile = freopen(log_filename,fopen_opts,fileHandle);
292+
logfile = freopen(log_filename, fopen_opts, file_handle);
300293

301-
running = (errcode == 0);
302294
flag_reopen_file = 0;
303295

304-
return 0;
296+
return logfile != 0;
305297
}
306298

307-
void get_logname_arg(const char * arg)
299+
static int process_logname_arg(const char *arg)
308300
{
309-
if(arg != 0)
310-
{
311-
size_t len = strnlen(arg,MAXLOGNAMESZ);
312-
if(len > 0 && len < MAXLOGNAMESZ)
313-
{
314-
strncpy(log_filename,arg,MAXLOGNAMESZ-1);
315-
}
316-
else
317-
{
318-
//print_usage(basename(argv[0]));
319-
exit(1);
301+
if (arg != 0) {
302+
const size_t len = strnlen(arg, MAXLOGNAMESZ);
303+
304+
if (len > 0 && len < MAXLOGNAMESZ) {
305+
strncpy(log_filename, arg, MAXLOGNAMESZ-1);
306+
} else {
307+
return 1;
320308
}
321309
is_auto_log_name = 0;
322-
}
323-
else
324-
{
310+
} else {
325311
is_auto_log_name = 1;
326-
sprint_auto_filename_format(&log_filename[0]);
312+
sprint_auto_filename_format(log_filename);
327313
}
314+
315+
return 0;
328316
}
329317

330318
int main(int argc, char **argv)
@@ -361,14 +349,12 @@ int main(int argc, char **argv)
361349
struct timeval tv, last_tv;
362350
struct timeval timeout, timeout_config = { 0, 0 }, *timeout_current = NULL;
363351

364-
{
365-
sigset_t sig_mask;
366-
sigemptyset (&sig_mask);
352+
sigset_t sig_mask;
353+
sigemptyset(&sig_mask);
367354

368-
struct sigaction sigterm_action = {.sa_mask = sig_mask, .sa_flags = SA_RESTART, .sa_handler = sigterm};
369-
sigaction (SIGHUP, &sigterm_action, NULL);
370-
sigaction (SIGINT, &sigterm_action, NULL);
371-
}
355+
struct sigaction sigterm_action = { .sa_mask = sig_mask, .sa_flags = SA_RESTART, .sa_handler = sigterm };
356+
sigaction (SIGHUP, &sigterm_action, NULL);
357+
sigaction (SIGINT, &sigterm_action, NULL);
372358

373359
last_tv.tv_sec = 0;
374360
last_tv.tv_usec = 0;
@@ -377,9 +363,8 @@ int main(int argc, char **argv)
377363

378364
//Since interface is a required argument, we don't need to parse opt for final arg
379365
//This enabled the -l option to take an optional filename
380-
if(getoptargc > 0)
381-
{
382-
getoptargc = argc-1;
366+
if (getoptargc > 0) {
367+
getoptargc = argc - 1;
383368
}
384369

385370
while ((opt = getopt(getoptargc, argv, ":t:HciaSs:l:DdxLn:r:he8T:?")) != -1) {
@@ -438,21 +423,26 @@ int main(int argc, char **argv)
438423
{
439424
log = 1;
440425

441-
get_logname_arg(optarg);
442-
426+
if (process_logname_arg(optarg) != 0) {
427+
print_usage(basename(argv[0]));
428+
exit(1);
429+
}
443430
break;
444431
}
445432
default:
446433
fprintf(stderr, "option -%c is missing a required argument\n", optopt);
447434
return EXIT_FAILURE;
448435
}
449-
450436
break;
451437

452438
case 'l':
453439
log = 1;
454440

455-
get_logname_arg(optarg);
441+
if (process_logname_arg(optarg) != 0) {
442+
is_auto_log_name = 0;
443+
print_usage(basename(argv[0]));
444+
exit(1);
445+
}
456446

457447
break;
458448

@@ -511,15 +501,13 @@ int main(int argc, char **argv)
511501
exit(0);
512502
}
513503

514-
//Configure SIGHUP handler to reopen file if logging
515-
{
516-
sigset_t sig_block_mask;
517-
sigfillset (&sig_block_mask);
518-
struct sigaction usr_action = {.sa_mask = sig_block_mask, .sa_flags = SA_RESTART};
519-
usr_action.sa_handler = log ? sighup : sigterm;
520-
sigaction(SIGHUP, &usr_action, NULL);
521-
}
522-
504+
/*Configure SIGHUP handler to reopen file if logging*/
505+
sigset_t sig_block_mask;
506+
sigfillset(&sig_block_mask);
507+
struct sigaction usr_action = { .sa_mask = sig_block_mask, .sa_flags = SA_RESTART };
508+
usr_action.sa_handler = log ? sighup : sigterm;
509+
sigaction(SIGHUP, &usr_action, NULL);
510+
523511
if (logfrmt && view) {
524512
fprintf(stderr, "Log file format selected: Please disable ASCII/BINARY/SWAP/RAWDLC options!\n");
525513
exit(0);
@@ -734,10 +722,8 @@ int main(int argc, char **argv)
734722
if (log) {
735723
int result = open_log_file(log_filename);
736724

737-
if(result != 0)
738-
{
725+
if (result != 0)
739726
return 1;
740-
}
741727
}
742728

743729
/* these settings are static and can be held out of the hot path */
@@ -759,20 +745,15 @@ int main(int argc, char **argv)
759745
if ((ret = select(s[currmax-1]+1, &rdfs, NULL, NULL, timeout_current)) <= 0) {
760746
//perror("select");
761747

762-
error_t serr = errno;
748+
const error_t serr = errno;
763749

764-
if(serr == EINTR)
765-
{
766-
if(flag_reopen_file == 1)
767-
{
768-
if(reopen_file(logfile) != 0)
769-
{
770-
return 1;
750+
if (serr == EINTR) {
751+
if (flag_reopen_file == 1) {
752+
if (reopen_file(logfile) != 0) {
753+
return -1;
771754
}
772755
}
773-
}
774-
else
775-
{
756+
} else {
776757
running = 0;
777758
}
778759

@@ -962,10 +943,8 @@ int main(int argc, char **argv)
962943
fflush(stdout);
963944
}
964945

965-
if(flag_reopen_file == 1)
966-
{
967-
if(reopen_file(logfile) != 0)
968-
{
946+
if (flag_reopen_file == 1) {
947+
if(reopen_file(logfile) != 0) {
969948
return -1;
970949
}
971950
}

0 commit comments

Comments
 (0)