[PATCH] D17639: [asan] Fix recvfrom.cc testcase failure in large parallel tests run.
Yury Gribov via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 26 06:38:25 PST 2016
ygribov added inline comments.
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:20
@@ +19,3 @@
+ exit(1); \
+ }
+
----------------
Maybe wrap in do-while(0) just in case?
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:30
@@ -23,4 +29,3 @@
int sockfd = socket(AF_INET, SOCK_DGRAM, 0);
- if (sockfd < 0)
- fprintf(stderr, "ERROR opening socket\n");
+ CHECK_ERROR(sockfd < 0, "ERROR opening socket.\n");
----------------
Still no agreement on ERROR vs. Error. I suggest to move prefix and newline to CHECK_ERROR macro.
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:35
@@ -29,2 +34,3 @@
serveraddr.sin_addr.s_addr = htonl(INADDR_ANY);
- serveraddr.sin_port = htons(kPortNum);
+ serveraddr.sin_port = htons(0);
+
----------------
Perhaps avoid htons?
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:45
@@ -35,1 +44,3 @@
+ // Now it's safe to unblock client thread.
+ pthread_mutex_unlock(&server_ready_mu);
recvfrom(sockfd, buf, kBufSize, 0, NULL, NULL); // BOOM
----------------
Shouldn't mutexes be checked as well? BTW you could probably avoid mutexes alltogether if you moved server to main thread (and start client thread from it after socket is initialized).
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:55
@@ -43,4 +54,3 @@
int main() {
- char buf[kBufSize] = "123456789";
- struct sockaddr_in serveraddr; // server's addr
-
+ char buf[kBufSize];
+ struct sockaddr_in serveraddr;
----------------
Should probably initialialize to avoid UB.
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:64
@@ +63,3 @@
+ int succeeded = pthread_create(&server_thread, NULL, server_thread_udp,
+ (void *)&serveraddr);
+ CHECK_ERROR(succeeded, "Error creating thread.\n");
----------------
I always thought that there's no need for explicit casts to starred voids (ditto for memset, memcpy, etc.).
================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:72
@@ -71,1 +71,3 @@
+ pthread_mutex_lock(&server_ready_mu);
+ sendto(sockfd, buf, sizeof(buf), 0, (struct sockaddr *)&serveraddr,
sizeof(serveraddr));
----------------
Should this be checked?
Repository:
rL LLVM
http://reviews.llvm.org/D17639
More information about the llvm-commits
mailing list