Skip to contentSkip to conversation

Return errors from the library consistently

PATCH In review Jason Gunthorpe 2 participants 2 messages Oct 20, 2009 – Oct 21, 2009 1 message not in the archive InfiniBand

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Return errors from the library consistently

Remove the return of -errno and always return codes via errno. As documented in librdmacm, these libraries are already documented to return -1 to indicate the code is in errno.

Update rping to show a correct error reporting methodology.

Also fix an errant return of 0 if the read/write syscalls return 0.

---
 examples/rping.c |   22 ++++++-------
 man/rdma_cm.7    |   16 ++++----
 src/cma.c        |   94 ++++++++++++++++++++++++++++-------------------------
 3 files changed, 68 insertions(+), 64 deletions(-)

I think the changes to rping show why the old approach is so dangerous, only 2 error reports in rping were actually able to show errno from the kernel.

I haven't been able to do more than compile test this one though..

examples/rping.c +10 -12
index e45cfb6..91952e7 100644
@@ -174,7 +174,7 @@ static int rping_cma_event_handler(struct rdma_cm_id *cma_id,
 		ret = rdma_resolve_route(cma_id, 2000);
 		if (ret) {
 			cb->state = ERROR;
-			fprintf(stderr, "rdma_resolve_route error %d\n", ret);
+			perror("rdma_resolve_route");
 			sem_post(&cb->sem);
 		}
 		break;
@@ -352,7 +352,7 @@ static int rping_accept(struct rping_cb *cb)
 
 	ret = rdma_accept(cb->child_cm_id, &conn_param);
 	if (ret) {
-		fprintf(stderr, "rdma_accept error: %d\n", ret);
+		perror("rdma_accept");
 		return ret;
 	}
 
@@ -546,7 +546,7 @@ static int rping_setup_qp(struct rping_cb *cb, struct rdma_cm_id *cm_id)
 
 	ret = rping_create_qp(cb);
 	if (ret) {
-		fprintf(stderr, "rping_create_qp failed: %d\n", ret);
+		perror("rdma_create_qp");
 		goto err3;
 	}
 	DEBUG_LOG("created qp %p\n", cb->qp);
@@ -570,7 +570,7 @@ static void *cm_thread(void *arg)
 	while (1) {
 		ret = rdma_get_cm_event(cb->cm_channel, &event);
 		if (ret) {
-			fprintf(stderr, "rdma_get_cm_event err %d\n", ret);
+			perror("rdma_get_cm_event");
 			exit(ret);
 		}
 		ret = rping_cma_event_handler(event->id, event);
@@ -736,7 +736,7 @@ static int rping_bind_server(struct rping_cb *cb)
 
 	ret = rdma_bind_addr(cb->cm_id, (struct sockaddr *) &cb->sin);
 	if (ret) {
-		fprintf(stderr, "rdma_bind_addr error %d\n", ret);
+		perror("rdma_bind_addr");
 		return ret;
 	}
 	DEBUG_LOG("rdma_bind_addr successful\n");
@@ -744,7 +744,7 @@ static int rping_bind_server(struct rping_cb *cb)
 	DEBUG_LOG("rdma_listen\n");
 	ret = rdma_listen(cb->cm_id, 3);
 	if (ret) {
-		fprintf(stderr, "rdma_listen failed: %d\n", ret);
+		perror("rdma_listen");
 		return ret;
 	}
 
@@ -978,7 +978,7 @@ static int rping_connect_client(struct rping_cb *cb)
 
 	ret = rdma_connect(cb->cm_id, &conn_param);
 	if (ret) {
-		fprintf(stderr, "rdma_connect error %d\n", ret);
+		perror("rdma_connect");
 		return ret;
 	}
 
@@ -1003,7 +1003,7 @@ static int rping_bind_client(struct rping_cb *cb)
 
 	ret = rdma_resolve_addr(cb->cm_id, NULL, (struct sockaddr *) &cb->sin, 2000);
 	if (ret) {
-		fprintf(stderr, "rdma_resolve_addr error %d\n", ret);
+		perror("rdma_resolve_addr");
 		return ret;
 	}
 
@@ -1191,15 +1191,13 @@ int main(int argc, char *argv[])
 
 	cb->cm_channel = rdma_create_event_channel();
 	if (!cb->cm_channel) {
-		ret = errno;
-		fprintf(stderr, "rdma_create_event_channel error %d\n", ret);
+		perror("rdma_create_event_channel");
 		goto out;
 	}
 
 	ret = rdma_create_id(cb->cm_channel, &cb->cm_id, cb, RDMA_PS_TCP);
 	if (ret) {
-		ret = errno;
-		fprintf(stderr, "rdma_create_id error %d\n", ret);
+		perror("rdma_create_id");
 		goto out2;
 	}
 	DEBUG_LOG("created cm_id %p\n", cb->cm_id);
man/rdma_cm.7 +8 -8
index 23571b2..fd04959 100644
@@ -108,18 +108,18 @@ release the event channel
 success
 .IP "= -1"
 error - see errno for more details
-.IP "< -1"
-error - see include/asm-generic/errno*.h for more details
 .P
-Librdmacm functions return 0 to indicate success, and a negative return value
+Librdmacm functions return 0 to indicate success, and a -1 return value
 to indicate failure.  If a function operates asynchronously, a return value of 0
 means that the operation was successfully started.  The operation could still
 complete in error; users should check the status of the related event.  If the
-return value is -1, then errno can be examined for additional information
-regarding the reason for the failure.  If the return value is < -1, then
-additional error reasons can be obtained by comparing the returned value with
-the values listed in include/asm-generic/errno-base.h and
-include/asm-generic/errno.h.
+return value is -1, then errno will contain additional information
+regarding the reason for the failure.
+.P
+Prior versions of the library would return -errno and not set errno for some cases
+related to ENOMEM, ENODEV, ENODATA, EINVAL, and EADDRNOTAVAIL codes. Applications
+that want to check these codes and have compatability with prior library versions
+must manually set errno to the negative of the return code if it is < -1.
 .SH "SEE ALSO"
 rdma_create_event_channel(3), rdma_get_cm_event(3), rdma_create_id(3),
 rdma_resolve_addr(3), rdma_bind_addr(3), rdma_create_qp(3),
src/cma.c +50 -44

351 lines not on this page · raw message

Signature · 6 lines
1.6.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20091020184157.GG14520-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

#2 lore raw
Quoted 9 lines

Remove the return of -errno and always return codes via errno. As documented in librdmacm, these libraries are already documented to return -1 to indicate the code is in errno.

Update rping to show a correct error reporting methodology.

Also fix an errant return of 0 if the read/write syscalls return 0.

Thanks - both of these look good to me and tested fine. I should push out new releases of both libraries by the end of the week with these changes and ask that they get pulled into OFED 1.5.

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html