From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Mon, 1 Dec 2008 07:24:18 +0000 (+0000)
Subject: Fix sloppy CAN_(EFF|RTR)_FLAG handling in can_filter.can_mask .
X-Git-Url: https://rtime.felk.cvut.cz/gitweb/sojka/can-utils.git/commitdiff_plain/6104197c6aa8c36a51b5ded080db433c1fed847a

Fix sloppy CAN_(EFF|RTR)_FLAG handling in can_filter.can_mask .

Due to a wrong safety check in af_can.c it was not possible to filter
for SFF frames with a specific CAN identifier without getting the
same selected CAN identifier from a received EFF frame also.

This fix has a minimum impact on the CAN filter API as the 'sloppy'
handling is still a correct (and possibly wanted?) use-case.

Please update the can-utils (especially candump) whose filter definition
on the commandline made assumptions to correct the user input that are
probably unwanted now.

Thanks to Kurt van Dijck for pointing at this issue!

Signed-Off-by: Oliver Hartkopp <oliver@hartkopp.net>
---

diff --git a/candump.c b/candump.c
index 940f81d..f2762f9 100644
--- a/candump.c
+++ b/candump.c
@@ -130,8 +130,9 @@ void print_usage(char *prg)
 	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n", prg);
 	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF    (log only error frames but no(!) data frames)\n", prg);
 	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF    (log error frames and also all data frames)\n", prg);
-	fprintf(stderr, "%s vcan2,92345678:9FFFFFFF (match only for extended CAN ID 12345678)\n", prg);
-	fprintf(stderr, "%s vcan2,12345678:1FFFFFFF (analog to above due to 8 digit value length)\n", prg);
+	fprintf(stderr, "%s vcan2,92345678:DFFFFFFF (match only for extended CAN ID 12345678)\n", prg);
+	fprintf(stderr, "%s vcan2,123:7FF (matches CAN ID 123 - including EFF and RTR frames)\n", prg);
+	fprintf(stderr, "%s vcan2,123:C00007FF (matches CAN ID 123 - only SFF and non-RTR frames)\n", prg);
 	fprintf(stderr, "\n");
 }
 
@@ -189,21 +190,6 @@ int idx2dindex(int ifidx, int socket) {
 	return i;
 }
 
-canid_t checkeff(char *ptr, char *nptr)
-{
-	int len;
-
-	if (nptr)
-		len = nptr - ptr;
-	else
-		len = strlen(ptr);
-
-	if (len == 17 && (ptr[8] == ':' || ptr[8] == '~'))
-		return CAN_EFF_FLAG;
-	else
-		return 0;
-}
-
 int main(int argc, char **argv)
 {
 	fd_set rdfs;
@@ -219,7 +205,6 @@ int main(int argc, char **argv)
 	int opt, ret;
 	int currmax, numfilter;
 	char *ptr, *nptr;
-	canid_t eff;
 	struct sockaddr_can addr;
 	struct can_filter rfilter[MAXFILTER];
 	can_err_mask_t err_mask;
@@ -411,9 +396,6 @@ int main(int argc, char **argv)
 					   &rfilter[numfilter].can_id, 
 					   (long unsigned int *)
 					   &rfilter[numfilter].can_mask) == 2) {
-					eff = checkeff(ptr, nptr);
- 					rfilter[numfilter].can_id |= eff;
- 					rfilter[numfilter].can_mask |= eff;
  					rfilter[numfilter].can_mask &= ~CAN_ERR_FLAG;
 					numfilter++;
 				} else if (sscanf(ptr, "%lx~%lx",
@@ -422,9 +404,6 @@ int main(int argc, char **argv)
 						  (long unsigned int *)
 						  &rfilter[numfilter].can_mask) == 2) {
  					rfilter[numfilter].can_id |= CAN_INV_FILTER;
-					eff = checkeff(ptr, nptr);
- 					rfilter[numfilter].can_id |= eff;
- 					rfilter[numfilter].can_mask |= eff;
  					rfilter[numfilter].can_mask &= ~CAN_ERR_FLAG;
 					numfilter++;
 				} else if (sscanf(ptr, "#%lx",
diff --git a/isotpdump.c b/isotpdump.c
index 22b8141..c5c4de5 100644
--- a/isotpdump.c
+++ b/isotpdump.c
@@ -162,17 +162,22 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	rfilter[0].can_id   = src;
-	if (src & CAN_EFF_FLAG)
-		rfilter[0].can_mask = CAN_EFF_MASK | CAN_EFF_FLAG;
-	else
-		rfilter[0].can_mask = CAN_SFF_MASK;
-
-	rfilter[1].can_id   = dst;
-	if (dst & CAN_EFF_FLAG)
-		rfilter[1].can_mask = CAN_EFF_MASK | CAN_EFF_FLAG;
-	else
-		rfilter[1].can_mask = CAN_SFF_MASK;
+
+	if (src & CAN_EFF_FLAG) {
+		rfilter[0].can_id   = src & (CAN_EFF_MASK | CAN_EFF_FLAG);
+		rfilter[0].can_mask = (CAN_EFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+	} else {
+		rfilter[0].can_id   = src & CAN_SFF_MASK;
+		rfilter[0].can_mask = (CAN_SFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+	}
+
+	if (dst & CAN_EFF_FLAG) {
+		rfilter[1].can_id   = dst & (CAN_EFF_MASK | CAN_EFF_FLAG);
+		rfilter[1].can_mask = (CAN_EFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+	} else {
+		rfilter[1].can_id   = dst & CAN_SFF_MASK;
+		rfilter[1].can_mask = (CAN_SFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+	}
 
 	setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER, &rfilter, sizeof(rfilter));