Skip to content

Commit 60be78c

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.
1 parent ec631ef commit 60be78c

File tree

1 file changed

+64
-86
lines changed

1 file changed

+64
-86
lines changed

Diff for: candump.c

+64-86
Original file line numberDiff line numberDiff line change
@@ -111,18 +111,18 @@ const char anichar[MAXANI] = {'|', '/', '-', '\\'};
111111
const char extra_m_info[4][4] = {"- -", "B -", "- E", "B E"};
112112

113113
#define MAXLOGNAMESZ 100
114-
FILE *logfile = NULL;
114+
static FILE *logfile = NULL;
115115
static char log_filename[MAXLOGNAMESZ];
116116

117-
unsigned char silent = SILENT_INI;
117+
static unsigned char silent = SILENT_INI;
118118

119119
extern int optind, opterr, optopt;
120120

121121
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);
122+
static volatile int flag_reopen_file;
123+
static volatile unsigned long sighup_count;
124+
125+
static int is_auto_log_name = 0;
126126

127127
void print_usage(char *prg)
128128
{
@@ -177,8 +177,7 @@ void sigterm(int signo)
177177

178178
void sighup(int signo)
179179
{
180-
if(signo == SIGHUP)
181-
{
180+
if (signo == SIGHUP && running) {
182181
flag_reopen_file = 1;
183182
sighup_count++;
184183
}
@@ -233,14 +232,13 @@ int idx2dindex(int ifidx, int socket) {
233232
return i;
234233
}
235234

236-
int sprint_auto_filename_format(char * buffer)
235+
static int sprint_auto_filename_format(char *buffer)
237236
{
238237
time_t currtime;
239238
struct tm now;
240239

241240
if (time(&currtime) == (time_t)-1) {
242241
perror("time");
243-
//running = 0;
244242
return 1;
245243
}
246244

@@ -259,14 +257,13 @@ int sprint_auto_filename_format(char * buffer)
259257
return 0;
260258
}
261259

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

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

271268
if (!logfile) {
272269
perror("logfile");
@@ -276,55 +273,45 @@ int open_log_file(const char * fileName)
276273
return 0;
277274
}
278275

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

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

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

291-
if(errcode != 0)
292-
{
293-
running = 0;
286+
if (errcode != 0) {
294287
return 1;
295288
}
296289
}
297290

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

301-
running = (errcode == 0);
302293
flag_reopen_file = 0;
303294

304-
return 0;
295+
return logfile != 0;
305296
}
306297

307-
void get_logname_arg(const char * arg)
298+
static int process_logname_arg(const char *arg)
308299
{
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);
300+
if (arg != 0) {
301+
const size_t len = strnlen(arg, MAXLOGNAMESZ);
302+
303+
if (len > 0 && len < MAXLOGNAMESZ) {
304+
strncpy(log_filename, arg, MAXLOGNAMESZ-1);
305+
} else {
306+
return 1;
320307
}
321308
is_auto_log_name = 0;
322-
}
323-
else
324-
{
309+
} else {
325310
is_auto_log_name = 1;
326-
sprint_auto_filename_format(&log_filename[0]);
311+
sprint_auto_filename_format(log_filename);
327312
}
313+
314+
return 0;
328315
}
329316

330317
int main(int argc, char **argv)
@@ -361,14 +348,12 @@ int main(int argc, char **argv)
361348
struct timeval tv, last_tv;
362349
struct timeval timeout, timeout_config = { 0, 0 }, *timeout_current = NULL;
363350

364-
{
365-
sigset_t sig_mask;
366-
sigemptyset (&sig_mask);
351+
sigset_t sig_mask;
352+
sigemptyset(&sig_mask);
367353

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-
}
354+
struct sigaction sigterm_action = { .sa_mask = sig_mask, .sa_flags = SA_RESTART, .sa_handler = sigterm };
355+
sigaction (SIGHUP, &sigterm_action, NULL);
356+
sigaction (SIGINT, &sigterm_action, NULL);
372357

373358
last_tv.tv_sec = 0;
374359
last_tv.tv_usec = 0;
@@ -377,9 +362,8 @@ int main(int argc, char **argv)
377362

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

385369
while ((opt = getopt(getoptargc, argv, ":t:HciaSs:l:DdxLn:r:he8T:?")) != -1) {
@@ -438,21 +422,26 @@ int main(int argc, char **argv)
438422
{
439423
log = 1;
440424

441-
get_logname_arg(optarg);
442-
425+
if (process_logname_arg(optarg) != 0) {
426+
print_usage(basename(argv[0]));
427+
exit(1);
428+
}
443429
break;
444430
}
445431
default:
446432
fprintf(stderr, "option -%c is missing a required argument\n", optopt);
447433
return EXIT_FAILURE;
448434
}
449-
450435
break;
451436

452437
case 'l':
453438
log = 1;
454439

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

457446
break;
458447

@@ -511,15 +500,13 @@ int main(int argc, char **argv)
511500
exit(0);
512501
}
513502

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-
503+
/*Configure SIGHUP handler to reopen file if logging*/
504+
sigset_t sig_block_mask;
505+
sigfillset(&sig_block_mask);
506+
struct sigaction usr_action = { .sa_mask = sig_block_mask, .sa_flags = SA_RESTART };
507+
usr_action.sa_handler = log ? sighup : sigterm;
508+
sigaction(SIGHUP, &usr_action, NULL);
509+
523510
if (logfrmt && view) {
524511
fprintf(stderr, "Log file format selected: Please disable ASCII/BINARY/SWAP/RAWDLC options!\n");
525512
exit(0);
@@ -734,10 +721,8 @@ int main(int argc, char **argv)
734721
if (log) {
735722
int result = open_log_file(log_filename);
736723

737-
if(result != 0)
738-
{
724+
if (result != 0)
739725
return 1;
740-
}
741726
}
742727

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

762-
error_t serr = errno;
747+
const int serr = errno;
763748

764-
if(serr == EINTR)
765-
{
766-
if(flag_reopen_file == 1)
767-
{
768-
if(reopen_file(logfile) != 0)
769-
{
770-
return 1;
749+
if (serr == EINTR) {
750+
if (flag_reopen_file == 1) {
751+
if (reopen_file(logfile) != 0) {
752+
return -1;
771753
}
772754
}
773-
}
774-
else
775-
{
755+
} else {
776756
running = 0;
777757
}
778758

@@ -962,10 +942,8 @@ int main(int argc, char **argv)
962942
fflush(stdout);
963943
}
964944

965-
if(flag_reopen_file == 1)
966-
{
967-
if(reopen_file(logfile) != 0)
968-
{
945+
if (flag_reopen_file == 1) {
946+
if(reopen_file(logfile) != 0) {
969947
return -1;
970948
}
971949
}

0 commit comments

Comments
 (0)