From 213a814da6692a788db90a2079dfcd1b00359cad Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 7 Dec 2022 15:26:42 +0100 Subject: [PATCH 01/16] =?UTF-8?q?cangen:=20don't=20compare=20floating-poin?= =?UTF-8?q?t=20gap=20with=20=E2=80=98=3D=3D=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the following warning: | cangen.c:524:7: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Wfloat-equal] | 524 | if (gap && burst_sent_count >= burst_count) /* gap == 0 => performance test :-] */ | | ^~~ Signed-off-by: Marc Kleine-Budde --- cangen.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index 2327064..8cb2fd2 100644 --- a/cangen.c +++ b/cangen.c @@ -521,7 +521,8 @@ resend: } burst_sent_count++; - if (gap && burst_sent_count >= burst_count) /* gap == 0 => performance test :-] */ + if ((ts.tv_sec || ts.tv_nsec) && + burst_sent_count >= burst_count) if (nanosleep(&ts, NULL)) return 1; From 144d77180eaa6a33785349544a7f26e223111ffd Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 10:44:25 +0200 Subject: [PATCH 02/16] cangen: checkpatch: don't split strings over multiple lines Signed-off-by: Marc Kleine-Budde --- cangen.c | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/cangen.c b/cangen.c index 8cb2fd2..d019c25 100644 --- a/cangen.c +++ b/cangen.c @@ -85,36 +85,23 @@ void print_usage(char *prg) fprintf(stderr, "%s - CAN frames generator.\n\n", prg); fprintf(stderr, "Usage: %s [options] \n", prg); fprintf(stderr, "Options:\n"); - fprintf(stderr, " -g (gap in milli seconds " - "- default: %d ms)\n", DEFAULT_GAP); - fprintf(stderr, " -e (generate extended frame mode " - "(EFF) CAN frames)\n"); + fprintf(stderr, " -g (gap in milli seconds - default: %d ms)\n", DEFAULT_GAP); + fprintf(stderr, " -e (generate extended frame mode (EFF) CAN frames)\n"); fprintf(stderr, " -f (generate CAN FD CAN frames)\n"); - fprintf(stderr, " -b (generate CAN FD CAN frames" - " with bitrate switch (BRS))\n"); - fprintf(stderr, " -E (generate CAN FD CAN frames" - " with error state (ESI))\n"); + fprintf(stderr, " -b (generate CAN FD CAN frames with bitrate switch (BRS))\n"); + fprintf(stderr, " -E (generate CAN FD CAN frames with error state (ESI))\n"); fprintf(stderr, " -R (generate RTR frames)\n"); fprintf(stderr, " -8 (allow DLC values greater then 8 for Classic CAN frames)\n"); fprintf(stderr, " -m (mix -e -f -b -E -R frames)\n"); - fprintf(stderr, " -I (CAN ID" - " generation mode - see below)\n"); - fprintf(stderr, " -L (CAN data length code (dlc)" - " generation mode - see below)\n"); - fprintf(stderr, " -D (CAN data (payload)" - " generation mode - see below)\n"); - fprintf(stderr, " -p (poll on -ENOBUFS to write frames" - " with ms)\n"); - fprintf(stderr, " -n (terminate after CAN frames " - "- default infinite)\n"); - fprintf(stderr, " -i (ignore -ENOBUFS return values on" - " write() syscalls)\n"); - fprintf(stderr, " -x (disable local loopback of " - "generated CAN frames)\n"); - fprintf(stderr, " -c (number of messages to send in burst, " - "default 1)\n"); - fprintf(stderr, " -v (increment verbose level for " - "printing sent CAN frames)\n\n"); + fprintf(stderr, " -I (CAN ID generation mode - see below)\n"); + fprintf(stderr, " -L (CAN data length code (dlc) generation mode - see below)\n"); + fprintf(stderr, " -D (CAN data (payload) generation mode - see below)\n"); + fprintf(stderr, " -p (poll on -ENOBUFS to write frames with ms)\n"); + fprintf(stderr, " -n (terminate after CAN frames - default infinite)\n"); + fprintf(stderr, " -i (ignore -ENOBUFS return values on write() syscalls)\n"); + fprintf(stderr, " -x (disable local loopback of generated CAN frames)\n"); + fprintf(stderr, " -c (number of messages to send in burst, default 1)\n"); + fprintf(stderr, " -v (increment verbose level for printing sent CAN frames)\n\n"); fprintf(stderr, "Generation modes:\n"); fprintf(stderr, " 'r' => random values (default)\n"); fprintf(stderr, " 'e' => random values, even ID\n"); @@ -122,8 +109,7 @@ void print_usage(char *prg) fprintf(stderr, " 'i' => increment values\n"); fprintf(stderr, " => fixed value (in hexadecimal for -I and -D)\n\n"); fprintf(stderr, "The gap value (in milliseconds) may have decimal places, e.g. '-g 4.73'\n"); - fprintf(stderr, "When incrementing the CAN data the data length code " - "minimum is set to 1.\n"); + fprintf(stderr, "When incrementing the CAN data the data length code minimum is set to 1.\n"); fprintf(stderr, "CAN IDs and data content are given and expected in hexadecimal values.\n\n"); fprintf(stderr, "Examples:\n"); fprintf(stderr, "%s vcan0 -g 4 -I 42A -L 1 -D i -v -v\n", prg); @@ -320,8 +306,7 @@ int main(int argc, char **argv) /* recognize obviously missing commandline option */ if (id_mode == MODE_FIX && frame.can_id > 0x7FF && !extended) { - printf("The given CAN-ID is greater than 0x7FF and " - "the '-e' option is not set.\n"); + printf("The given CAN-ID is greater than 0x7FF and the '-e' option is not set.\n"); return 1; } From 77e785b022113932c978888207520c231a514ab4 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:02:46 +0200 Subject: [PATCH 03/16] cangen: checkpatch: fix space and newline usage Signed-off-by: Marc Kleine-Budde --- cangen.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/cangen.c b/cangen.c index d019c25..3123f20 100644 --- a/cangen.c +++ b/cangen.c @@ -249,7 +249,7 @@ int main(int argc, char **argv) } else { data_mode = MODE_FIX; if (hexstring2data(optarg, fixdata, CANFD_MAX_DLEN)) { - printf ("wrong fix data definition\n"); + printf("wrong fix data definition\n"); return 1; } } @@ -357,7 +357,7 @@ int main(int argc, char **argv) } /* interface is ok - try to switch the socket into CAN FD mode */ - if (setsockopt(s, SOL_CAN_RAW, CAN_RAW_FD_FRAMES, &enable_canfd, sizeof(enable_canfd))){ + if (setsockopt(s, SOL_CAN_RAW, CAN_RAW_FD_FRAMES, &enable_canfd, sizeof(enable_canfd))) { printf("error when enabling CAN FD support\n"); return 1; } @@ -394,7 +394,7 @@ int main(int argc, char **argv) if (count && (--count == 0)) running = 0; - if (canfd){ + if (canfd) { mtu = CANFD_MTU; maxdlen = CANFD_MAX_DLEN; if (brs) @@ -423,7 +423,6 @@ int main(int argc, char **argv) frame.can_id |= CAN_RTR_FLAG; if (dlc_mode == MODE_RANDOM) { - if (canfd) frame.len = can_fd_dlc2len(random() & 0xF); else { @@ -445,7 +444,6 @@ int main(int argc, char **argv) frame.len = 1; /* min dlc value for incr. data */ if (data_mode == MODE_RANDOM) { - rnd = random(); memcpy(&frame.data[0], &rnd, 4); rnd = random(); @@ -467,11 +465,10 @@ int main(int argc, char **argv) memset(&frame.data[frame.len], 0, maxdlen - frame.len); if (verbose) { - printf(" %s ", argv[optind]); if (verbose > 1) - fprint_long_canframe(stdout, &frame, "\n", (verbose > 2)?1:0, maxdlen); + fprint_long_canframe(stdout, &frame, "\n", (verbose > 2) ? 1 : 0, maxdlen); else fprint_canframe(stdout, &frame, "\n", 1, maxdlen); } @@ -518,7 +515,6 @@ resend: frame.can_id++; if (dlc_mode == MODE_INCREMENT) { - incdlc++; incdlc %= CAN_MAX_RAW_DLC + 1; @@ -539,22 +535,21 @@ resend: } if (data_mode == MODE_INCREMENT) { - incdata++; - for (i=0; i<8 ;i++) - frame.data[i] = (incdata >> i*8) & 0xFFULL; + for (i = 0; i < 8; i++) + frame.data[i] = (incdata >> i * 8) & 0xFFULL; } if (mix) { i = random(); - extended = i&1; - canfd = i&2; + extended = i & 1; + canfd = i & 2; if (canfd) { - brs = i&4; - esi = i&8; + brs = i & 4; + esi = i & 8; } - rtr_frame = ((i&24) == 24); /* reduce RTR frames to 1/4 */ + rtr_frame = ((i & 24) == 24); /* reduce RTR frames to 1/4 */ } } From 1d55d085c975688e9146d54e02fcb1496b002e31 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:04:26 +0200 Subject: [PATCH 04/16] cangen: checkpatch: fix comment style Signed-off-by: Marc Kleine-Budde --- cangen.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cangen.c b/cangen.c index 3123f20..87d095a 100644 --- a/cangen.c +++ b/cangen.c @@ -329,10 +329,12 @@ int main(int argc, char **argv) } addr.can_ifindex = ifr.ifr_ifindex; - /* disable default receive filter on this RAW socket */ - /* This is obsolete as we do not read from the socket at all, but for */ - /* this reason we can remove the receive list in the Kernel to save a */ - /* little (really a very little!) CPU usage. */ + /* + * disable default receive filter on this RAW socket + * This is obsolete as we do not read from the socket at all, but for + * this reason we can remove the receive list in the Kernel to save a + * little (really a very little!) CPU usage. + */ setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER, NULL, 0); if (loopback_disable) { From ad638db75daf8f97053a036f31a29c006b9fdf34 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:19:10 +0200 Subject: [PATCH 05/16] cangen: checkpatch: remove break after return Signed-off-by: Marc Kleine-Budde --- cangen.c | 1 - 1 file changed, 1 deletion(-) diff --git a/cangen.c b/cangen.c index 87d095a..a9175a1 100644 --- a/cangen.c +++ b/cangen.c @@ -292,7 +292,6 @@ int main(int argc, char **argv) default: print_usage(basename(argv[0])); return 1; - break; } } From 59c87f149bac152d56a8e521507bf916273aa857 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:19:36 +0200 Subject: [PATCH 06/16] cangen: checkpatch: don't assign in if statement Signed-off-by: Marc Kleine-Budde --- cangen.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index a9175a1..90333c9 100644 --- a/cangen.c +++ b/cangen.c @@ -314,7 +314,8 @@ int main(int argc, char **argv) return 1; } - if ((s = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) { + s = socket(PF_CAN, SOCK_RAW, CAN_RAW); + if (s < 0) { perror("socket"); return 1; } From 8d493edbbe0c0b18daed5a6371c688b3713b3675 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:47:49 +0200 Subject: [PATCH 07/16] cangen: checkpatch: put braces on all arms of if statement Signed-off-by: Marc Kleine-Budde --- cangen.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index 90333c9..f101f85 100644 --- a/cangen.c +++ b/cangen.c @@ -496,8 +496,9 @@ resend: return 1; } goto resend; - } else + } else { enobufs_count++; + } } else if (nbytes < mtu) { fprintf(stderr, "write: incomplete CAN frame\n"); From 11a0f19244f38fa1be4a2109eb42a4c69cf820d3 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 21 Apr 2022 13:56:17 +0200 Subject: [PATCH 08/16] cangen: properly initialize struct sockaddr_can addr Signed-off-by: Marc Kleine-Budde --- cangen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index f101f85..9f4579b 100644 --- a/cangen.c +++ b/cangen.c @@ -163,7 +163,7 @@ int main(int argc, char **argv) int s; /* socket */ struct pollfd fds; - struct sockaddr_can addr; + struct sockaddr_can addr = { 0 }; static struct canfd_frame frame; struct can_frame *ccf = (struct can_frame *)&frame; int nbytes; From a130ab5e8d5bc2d31bf1c660e296ea64df1f2123 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 10:47:04 +0200 Subject: [PATCH 09/16] cangen: mark functions as static Signed-off-by: Marc Kleine-Budde --- cangen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cangen.c b/cangen.c index 9f4579b..42354b1 100644 --- a/cangen.c +++ b/cangen.c @@ -80,7 +80,7 @@ extern int optind, opterr, optopt; static volatile int running = 1; static unsigned long long enobufs_count; -void print_usage(char *prg) +static void print_usage(char *prg) { fprintf(stderr, "%s - CAN frames generator.\n\n", prg); fprintf(stderr, "Usage: %s [options] \n", prg); @@ -128,7 +128,7 @@ void print_usage(char *prg) fprintf(stderr, "\t(my favourite default :)\n\n"); } -void sigterm(int signo) +static void sigterm(int signo) { running = 0; } From 0f7c1aa23a0a669f23b2c86c4ca4a7287fc49983 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 11:49:13 +0200 Subject: [PATCH 10/16] cangen: use consistent indention scheme of 1 space Signed-off-by: Marc Kleine-Budde --- cangen.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cangen.c b/cangen.c index 42354b1..5f7c163 100644 --- a/cangen.c +++ b/cangen.c @@ -69,11 +69,11 @@ #define DEFAULT_GAP 200 /* ms */ #define DEFAULT_BURST_COUNT 1 -#define MODE_RANDOM 0 -#define MODE_INCREMENT 1 -#define MODE_FIX 2 -#define MODE_RANDOM_EVEN 3 -#define MODE_RANDOM_ODD 4 +#define MODE_RANDOM 0 +#define MODE_INCREMENT 1 +#define MODE_FIX 2 +#define MODE_RANDOM_EVEN 3 +#define MODE_RANDOM_ODD 4 extern int optind, opterr, optopt; From 1f96d674c0cb7b60ee315981f12a7feb94829697 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 10:53:18 +0200 Subject: [PATCH 11/16] cangen: use if_nametoindex() to avoid overflows This patch replaces strcpy() + ioctl() by if_nametoindex() to avoid overflows caused by long user input. Signed-off-by: Marc Kleine-Budde --- cangen.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cangen.c b/cangen.c index 5f7c163..963f9c5 100644 --- a/cangen.c +++ b/cangen.c @@ -321,13 +321,11 @@ int main(int argc, char **argv) } addr.can_family = AF_CAN; - - strcpy(ifr.ifr_name, argv[optind]); - if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) { - perror("SIOCGIFINDEX"); + addr.can_ifindex = if_nametoindex(argv[optind]); + if (!addr.can_ifindex) { + perror("if_nametoindex"); return 1; } - addr.can_ifindex = ifr.ifr_ifindex; /* * disable default receive filter on this RAW socket From 29b05de39d4c5444ab15b72865bdd67d99d594f6 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 14:01:05 +0200 Subject: [PATCH 12/16] cangen: mark setsockopt() options as const Signed-off-by: Marc Kleine-Budde --- cangen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cangen.c b/cangen.c index 963f9c5..0c65fdc 100644 --- a/cangen.c +++ b/cangen.c @@ -336,14 +336,14 @@ int main(int argc, char **argv) setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER, NULL, 0); if (loopback_disable) { - int loopback = 0; + const int loopback = 0; setsockopt(s, SOL_CAN_RAW, CAN_RAW_LOOPBACK, &loopback, sizeof(loopback)); } if (canfd) { - int enable_canfd = 1; + const int enable_canfd = 1; /* check if the frame fits into the CAN netdevice */ if (ioctl(s, SIOCGIFMTU, &ifr) < 0) { From 8fb0e954b3a48919f78a3f8fdf8df3440c53278d Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 14:02:56 +0200 Subject: [PATCH 13/16] cangen: remove unneeded masking Signed-off-by: Marc Kleine-Budde --- cangen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index 0c65fdc..892f704 100644 --- a/cangen.c +++ b/cangen.c @@ -539,7 +539,7 @@ resend: incdata++; for (i = 0; i < 8; i++) - frame.data[i] = (incdata >> i * 8) & 0xFFULL; + frame.data[i] = incdata >> i * 8; } if (mix) { From 4b6f8d62ddad0c6f6c1acd0c63fdc5b65c50656a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 14:43:02 +0200 Subject: [PATCH 14/16] cangen: move scope of variable ret ... so that it can be used in other parts of the main() functions, too. Signed-off-by: Marc Kleine-Budde --- cangen.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cangen.c b/cangen.c index 892f704..f665547 100644 --- a/cangen.c +++ b/cangen.c @@ -172,6 +172,7 @@ int main(int argc, char **argv) struct timespec ts; struct timeval now; + int ret; /* set seed value for pseudo random numbers */ gettimeofday(&now, NULL); @@ -485,8 +486,6 @@ resend: return 1; } if (polltimeout) { - int ret; - /* wait for the write socket (with timeout) */ ret = poll(&fds, 1, polltimeout); if (ret == 0 || (ret == -1 && errno != -EINTR)) { From 99686af630b5806270f1e216d238514f61e2230a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 14:43:59 +0200 Subject: [PATCH 15/16] cangen: print_usage() don't hardcode default burst size Signed-off-by: Marc Kleine-Budde --- cangen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cangen.c b/cangen.c index f665547..00f14fd 100644 --- a/cangen.c +++ b/cangen.c @@ -100,7 +100,7 @@ static void print_usage(char *prg) fprintf(stderr, " -n (terminate after CAN frames - default infinite)\n"); fprintf(stderr, " -i (ignore -ENOBUFS return values on write() syscalls)\n"); fprintf(stderr, " -x (disable local loopback of generated CAN frames)\n"); - fprintf(stderr, " -c (number of messages to send in burst, default 1)\n"); + fprintf(stderr, " -c (number of messages to send in burst, default %u)\n", DEFAULT_BURST_COUNT); fprintf(stderr, " -v (increment verbose level for printing sent CAN frames)\n\n"); fprintf(stderr, "Generation modes:\n"); fprintf(stderr, " 'r' => random values (default)\n"); From b6a65b8105a37605e63ac99435d7a548bf617048 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 27 Mar 2022 12:35:13 +0200 Subject: [PATCH 16/16] cangen: sort getopt() by order of option in usage Signed-off-by: Marc Kleine-Budde --- cangen.c | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/cangen.c b/cangen.c index 00f14fd..e1659fc 100644 --- a/cangen.c +++ b/cangen.c @@ -182,13 +182,9 @@ int main(int argc, char **argv) signal(SIGHUP, sigterm); signal(SIGINT, sigterm); - while ((opt = getopt(argc, argv, "ig:ebEfmI:L:D:xp:n:c:vR8h?")) != -1) { + while ((opt = getopt(argc, argv, "g:efbER8mI:L:D:p:n:ixc:vh?")) != -1) { switch (opt) { - case 'i': - ignore_enobufs = 1; - break; - case 'g': gap = strtod(optarg, NULL); break; @@ -211,6 +207,14 @@ int main(int argc, char **argv) canfd = 1; break; + case 'R': + rtr_frame = 1; + break; + + case '8': + len8_dlc = 1; + break; + case 'm': mix = 1; canfd = 1; /* to switch the socket into CAN FD mode */ @@ -256,26 +260,6 @@ int main(int argc, char **argv) } break; - case 'c': - burst_count = strtoul(optarg, NULL, 10); - break; - - case 'v': - verbose++; - break; - - case 'x': - loopback_disable = 1; - break; - - case 'R': - rtr_frame = 1; - break; - - case '8': - len8_dlc = 1; - break; - case 'p': polltimeout = strtoul(optarg, NULL, 10); break; @@ -288,6 +272,22 @@ int main(int argc, char **argv) } break; + case 'i': + ignore_enobufs = 1; + break; + + case 'x': + loopback_disable = 1; + break; + + case 'c': + burst_count = strtoul(optarg, NULL, 10); + break; + + case 'v': + verbose++; + break; + case '?': case 'h': default: