kgio RubyGem user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
From: Eric Wong <normalperson@yhbt.net>
To: kgio@librelist.org
Subject: [PATCH 1/2] check syscall returns against < 0 instead of == -1
Date: Fri, 16 Aug 2013 02:09:27 +0000	[thread overview]
Message-ID: <1376618968-13492-2-git-send-email-normalperson@yhbt.net> (raw)
In-Reply-To: 1376618968-13492-1-git-send-email-normalperson@yhbt.net

This may help us avoid errors in case of C library bugs,
and also results in smaller code:

$ ~/linux/scripts/bloat-o-meter before.so after.so
add/remove: 0/0 grow/shrink: 0/9 up/down: 0/-66 (-66)
function                                     old     new   delta
s_trywrite                                   160     159      -1
kgio_write                                   160     159      -1
kgio_trywrite                                160     159      -1
my_recv                                      616     610      -6
my_peek                                      616     610      -6
stream_connect                               456     448      -8
my_socket                                    222     213      -9
my_writev                                   1703    1687     -16
write_check                                  427     409     -18
---
 ext/kgio/accept.c     | 4 ++--
 ext/kgio/connect.c    | 8 ++++----
 ext/kgio/nonblock.h   | 6 +++++-
 ext/kgio/read_write.c | 6 +++---
 ext/kgio/tryopen.c    | 4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/ext/kgio/accept.c b/ext/kgio/accept.c
index 5f07532..503b5e9 100644
--- a/ext/kgio/accept.c
+++ b/ext/kgio/accept.c
@@ -68,7 +68,7 @@ static VALUE xaccept(void *ptr)
 	int rv;
 
 	rv = accept_fn(a->fd, a->addr, a->addrlen, a->flags);
-	if (rv == -1 && errno == ENOSYS && accept_fn != my_accept4) {
+	if (rv < 0 && errno == ENOSYS && accept_fn != my_accept4) {
 		accept_fn = my_accept4;
 		rv = accept_fn(a->fd, a->addr, a->addrlen, a->flags);
 	}
@@ -170,7 +170,7 @@ my_accept(struct accept_args *a, int force_nonblock)
 
 retry:
 	client_fd = thread_accept(a, force_nonblock);
-	if (client_fd == -1) {
+	if (client_fd < 0) {
 		switch (errno) {
 		case EAGAIN:
 			if (force_nonblock)
diff --git a/ext/kgio/connect.c b/ext/kgio/connect.c
index 63c0bbc..2027795 100644
--- a/ext/kgio/connect.c
+++ b/ext/kgio/connect.c
@@ -36,7 +36,7 @@ static int my_socket(int domain)
 retry:
 	fd = socket(domain, MY_SOCK_STREAM, 0);
 
-	if (fd == -1) {
+	if (fd < 0) {
 		switch (errno) {
 		case EMFILE:
 		case ENFILE:
@@ -53,12 +53,12 @@ retry:
 				goto retry;
 			}
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("socket");
 	}
 
 	if (MY_SOCK_STREAM == SOCK_STREAM) {
-		if (fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK) == -1)
+		if (fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK) < 0)
 			close_fail(fd, "fcntl(F_SETFL, O_RDWR | O_NONBLOCK)");
 		rb_fd_fix_cloexec(fd);
 	}
@@ -71,7 +71,7 @@ my_connect(VALUE klass, int io_wait, int domain, void *addr, socklen_t addrlen)
 {
 	int fd = my_socket(domain);
 
-	if (connect(fd, addr, addrlen) == -1) {
+	if (connect(fd, addr, addrlen) < 0) {
 		if (errno == EINPROGRESS) {
 			VALUE io = sock_for_fd(klass, fd);
 
diff --git a/ext/kgio/nonblock.h b/ext/kgio/nonblock.h
index 43de8cd..2d9a13a 100644
--- a/ext/kgio/nonblock.h
+++ b/ext/kgio/nonblock.h
@@ -5,11 +5,15 @@ static void set_nonblocking(int fd)
 {
 	int flags = fcntl(fd, F_GETFL);
 
+	/*
+	 * do not check < 0 here, one day we may have enough FD flags
+	 * to require negative bit
+	 */
 	if (flags == -1)
 		rb_sys_fail("fcntl(F_GETFL)");
 	if ((flags & O_NONBLOCK) == O_NONBLOCK)
 		return;
 	flags = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
-	if (flags == -1)
+	if (flags < 0)
 		rb_sys_fail("fcntl(F_SETFL)");
 }
diff --git a/ext/kgio/read_write.c b/ext/kgio/read_write.c
index 4ce3541..f4d658c 100644
--- a/ext/kgio/read_write.c
+++ b/ext/kgio/read_write.c
@@ -109,7 +109,7 @@ static void prepare_read(struct io_args *a, int argc, VALUE *argv, VALUE io)
 
 static int read_check(struct io_args *a, long n, const char *msg, int io_wait)
 {
-	if (n == -1) {
+	if (n < 0) {
 		if (errno == EINTR) {
 			a->fd = my_fileno(a->io);
 			return -1;
@@ -344,7 +344,7 @@ static int write_check(struct io_args *a, long n, const char *msg, int io_wait)
 	if (a->len == n) {
 done:
 		a->buf = Qnil;
-	} else if (n == -1) {
+	} else if (n < 0) {
 		if (errno == EINTR) {
 			a->fd = my_fileno(a->io);
 			return -1;
@@ -589,7 +589,7 @@ static int writev_check(struct io_args_v *a, long n, const char *msg, int io_wai
 	if (n >= 0) {
 		if (n > 0) a->something_written = 1;
 		return trim_writev_buffer(a, n);
-	} else if (n == -1) {
+	} else if (n < 0) {
 		if (errno == EINTR) {
 			a->fd = my_fileno(a->io);
 			return -1;
diff --git a/ext/kgio/tryopen.c b/ext/kgio/tryopen.c
index 0ee6a54..5fa6ada 100644
--- a/ext/kgio/tryopen.c
+++ b/ext/kgio/tryopen.c
@@ -106,7 +106,7 @@ static VALUE s_tryopen(int argc, VALUE *argv, VALUE klass)
 
 retry:
 	fd = (int)rb_thread_blocking_region(nogvl_open, &o, RUBY_UBF_IO, 0);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM) {
 			rb_gc();
 			if (retried)
@@ -114,7 +114,7 @@ retry:
 			retried = 1;
 			goto retry;
 		}
-		if (fd == -1) {
+		if (fd < 0) {
 			int saved_errno = errno;
 
 			if (!st_lookup(errno2sym, (st_data_t)errno, &rv)) {
-- 
1.8.2.rc3.2.geae6cf5



  reply	other threads:[~2013-08-16  2:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-16  2:09 [PATCH 0/2] minor tweaks for potentially buggy platforms Eric Wong
2013-08-16  2:09 ` Eric Wong [this message]
2013-08-16  2:09 ` [PATCH 2/2] accept: more informative exception on unknown family Eric Wong
2013-08-16  2:15 ` [PATCH 0/2] minor tweaks for potentially buggy platforms Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/kgio/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1376618968-13492-2-git-send-email-normalperson@yhbt.net \
    --to=normalperson@yhbt.net \
    --cc=kgio@librelist.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhbt.net/kgio.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).