sleepy_penguin RubyGem user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
* [sleepy.penguin] [PATCH] favor comparison against 0 for error checking
@ 2013-04-21  3:09 Eric Wong
  0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2013-04-21  3:09 UTC (permalink / raw)
  To: sleepy.penguin

When possible, comparisons against zero require one less load
and uses one less register, so this results in smaller code:

  $ ~/linux/scripts/bloat-o-meter before.so after.so
  add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-57 (-57)
  function                                     old     new   delta
  rm_watch                                      84      83      -1
  rb_sp_set_nonblock                            80      79      -1
  add_watch                                    127     126      -1
  epwait                                       692     687      -5
  s_new                                        970     921     -49

This style is favored by major C projects, including glibc.

Note: since file and file descriptor flags may eventually use
more bits of an integer, we continue comparing F_GETFD/F_GETFL
return values against -1 to be future proof.
---
 ext/sleepy_penguin/epoll.c           |  6 +++---
 ext/sleepy_penguin/eventfd.c         |  8 ++++----
 ext/sleepy_penguin/inotify.c         |  8 ++++----
 ext/sleepy_penguin/missing_epoll.h   |  2 +-
 ext/sleepy_penguin/missing_inotify.h |  4 ++--
 ext/sleepy_penguin/signalfd.c        |  8 ++++----
 ext/sleepy_penguin/timerfd.c         | 10 +++++-----
 ext/sleepy_penguin/util.c            |  2 +-
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/ext/sleepy_penguin/epoll.c b/ext/sleepy_penguin/epoll.c
index ca19786..8e49171 100644
--- a/ext/sleepy_penguin/epoll.c
+++ b/ext/sleepy_penguin/epoll.c
@@ -106,7 +106,7 @@ static VALUE s_new(VALUE klass, VALUE _flags)
 			rb_gc();
 			fd = epoll_create1(flags);
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("epoll_create1");
 	}
 
@@ -150,7 +150,7 @@ static VALUE epwait_result(struct ep_per_thread *ept, int n)
 	struct epoll_event *epoll_event = ept->events;
 	VALUE obj_events, obj;
 
-	if (n == -1)
+	if (n < 0)
 		rb_sys_fail("epoll_wait");
 
 	for (i = n; --i >= 0; epoll_event++) {
@@ -192,7 +192,7 @@ static VALUE real_epwait(struct ep_per_thread *ept)
 
 	do {
 		n = (long)rb_sp_fd_region(nogvl_wait, ept, ept->fd);
-	} while (n == -1 && epoll_resume_p(expire_at, ept));
+	} while (n < 0 && epoll_resume_p(expire_at, ept));
 
 	return epwait_result(ept, (int)n);
 }
diff --git a/ext/sleepy_penguin/eventfd.c b/ext/sleepy_penguin/eventfd.c
index 1713fdd..4f95b0d 100644
--- a/ext/sleepy_penguin/eventfd.c
+++ b/ext/sleepy_penguin/eventfd.c
@@ -30,12 +30,12 @@ static VALUE s_new(int argc, VALUE *argv, VALUE klass)
 	flags = rb_sp_get_flags(klass, _flags, RB_SP_CLOEXEC(EFD_CLOEXEC));
 
 	fd = eventfd(initval, flags);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM) {
 			rb_gc();
 			fd = eventfd(initval, flags);
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("eventfd");
 	}
 
@@ -88,7 +88,7 @@ static VALUE incr(int argc, VALUE *argv, VALUE self)
 	x.val = (uint64_t)NUM2ULL(value);
 retry:
 	w = (ssize_t)rb_sp_fd_region(efd_write, &x, x.fd);
-	if (w == -1) {
+	if (w < 0) {
 		if (errno == EAGAIN && RTEST(nonblock))
 			return Qfalse;
 		if (rb_sp_wait(rb_io_wait_writable, self, &x.fd))
@@ -124,7 +124,7 @@ static VALUE getvalue(int argc, VALUE *argv, VALUE self)
 	RTEST(nonblock) ? rb_sp_set_nonblock(x.fd) : blocking_io_prepare(x.fd);
 retry:
 	w = (ssize_t)rb_sp_fd_region(efd_read, &x, x.fd);
-	if (w == -1) {
+	if (w < 0) {
 		if (errno == EAGAIN && RTEST(nonblock))
 			return Qnil;
 		if (rb_sp_wait(rb_io_wait_readable, self, &x.fd))
diff --git a/ext/sleepy_penguin/inotify.c b/ext/sleepy_penguin/inotify.c
index 6155507..4e771a2 100644
--- a/ext/sleepy_penguin/inotify.c
+++ b/ext/sleepy_penguin/inotify.c
@@ -30,12 +30,12 @@ static VALUE s_new(int argc, VALUE *argv, VALUE klass)
 	flags = rb_sp_get_flags(klass, _flags, RB_SP_CLOEXEC(IN_CLOEXEC));
 
 	fd = inotify_init1(flags);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM) {
 			rb_gc();
 			fd = inotify_init1(flags);
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("inotify_init1");
 	}
 
@@ -93,7 +93,7 @@ static VALUE add_watch(VALUE self, VALUE path, VALUE vmask)
 	uint32_t mask = rb_sp_get_uflags(self, vmask);
 	int rc = inotify_add_watch(fd, pathname, mask);
 
-	if (rc == -1)
+	if (rc < 0)
 		rb_sys_fail("inotify_add_watch");
 
 	return UINT2NUM((uint32_t)rc);
@@ -112,7 +112,7 @@ static VALUE rm_watch(VALUE self, VALUE vwd)
 	int fd = rb_sp_fileno(self);
 	int rc = inotify_rm_watch(fd, wd);
 
-	if (rc == -1)
+	if (rc < 0)
 		rb_sys_fail("inotify_rm_watch");
 	return INT2NUM(rc);
 }
diff --git a/ext/sleepy_penguin/missing_epoll.h b/ext/sleepy_penguin/missing_epoll.h
index 09b19d3..aa55d3a 100644
--- a/ext/sleepy_penguin/missing_epoll.h
+++ b/ext/sleepy_penguin/missing_epoll.h
@@ -15,7 +15,7 @@ static int my_epoll_create1(int flags)
 	if (fd < 0 || flags == 0)
 		return fd;
 
-	if ((flags & EPOLL_CLOEXEC) && (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1))
+	if ((flags & EPOLL_CLOEXEC) && (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0))
 		goto err;
 	return fd;
 err:
diff --git a/ext/sleepy_penguin/missing_inotify.h b/ext/sleepy_penguin/missing_inotify.h
index 49b324e..e22250e 100644
--- a/ext/sleepy_penguin/missing_inotify.h
+++ b/ext/sleepy_penguin/missing_inotify.h
@@ -36,13 +36,13 @@ static int my_inotify_init1(int flags)
 
 	if (fd < 0)
 		return fd;
-	if ((flags & IN_CLOEXEC) && (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1))
+	if ((flags & IN_CLOEXEC) && (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0))
 		goto fcntl_err;
 	if (flags & IN_NONBLOCK) {
 		tmp = fcntl(fd, F_GETFL);
 		if (tmp == -1)
 			goto fcntl_err;
-		if ((fcntl(fd, F_SETFL, tmp | O_NONBLOCK) != 0))
+		if ((fcntl(fd, F_SETFL, tmp | O_NONBLOCK) < 0))
 			goto fcntl_err;
 	}
 
diff --git a/ext/sleepy_penguin/signalfd.c b/ext/sleepy_penguin/signalfd.c
index ac5db68..511745c 100644
--- a/ext/sleepy_penguin/signalfd.c
+++ b/ext/sleepy_penguin/signalfd.c
@@ -97,7 +97,7 @@ static VALUE update_bang(int argc, VALUE *argv, VALUE self)
 	value2sigset(&mask, vmask);
 
 	rc = signalfd(fd, &mask, flags);
-	if (rc == -1)
+	if (rc < 0)
 		rb_sys_fail("signalfd");
 	return self;
 }
@@ -133,12 +133,12 @@ static VALUE s_new(int argc, VALUE *argv, VALUE klass)
 	value2sigset(&mask, vmask);
 
 	fd = signalfd(-1, &mask, flags);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM) {
 			rb_gc();
 			fd = signalfd(-1, &mask, flags);
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("signalfd");
 	}
 
@@ -196,7 +196,7 @@ static VALUE sfd_take(int argc, VALUE *argv, VALUE self)
 retry:
 	ssi->ssi_fd = fd;
 	r = (ssize_t)rb_sp_fd_region(sfd_read, ssi, fd);
-	if (r == -1) {
+	if (r < 0) {
 		if (errno == EAGAIN && RTEST(nonblock))
 			return Qnil;
 		if (rb_sp_wait(rb_io_wait_readable, self, &fd))
diff --git a/ext/sleepy_penguin/timerfd.c b/ext/sleepy_penguin/timerfd.c
index 33ef8a7..a4f5c7d 100644
--- a/ext/sleepy_penguin/timerfd.c
+++ b/ext/sleepy_penguin/timerfd.c
@@ -30,12 +30,12 @@ static VALUE s_new(int argc, VALUE *argv, VALUE klass)
 	flags = rb_sp_get_flags(klass, fl, RB_SP_CLOEXEC(TFD_CLOEXEC));
 
 	fd = timerfd_create(clockid, flags);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM) {
 			rb_gc();
 			fd = timerfd_create(clockid, flags);
 		}
-		if (fd == -1)
+		if (fd < 0)
 			rb_sys_fail("timerfd_create");
 	}
 
@@ -72,7 +72,7 @@ static VALUE settime(VALUE self, VALUE fl, VALUE interval, VALUE value)
 	value2timespec(&new.it_interval, interval);
 	value2timespec(&new.it_value, value);
 
-	if (timerfd_settime(fd, flags, &new, &old) == -1)
+	if (timerfd_settime(fd, flags, &new, &old) < 0)
 		rb_sys_fail("timerfd_settime");
 
 	return itimerspec2ary(&old);
@@ -89,7 +89,7 @@ static VALUE gettime(VALUE self)
 	int fd = rb_sp_fileno(self);
 	struct itimerspec curr;
 
-	if (timerfd_gettime(fd, &curr) == -1)
+	if (timerfd_gettime(fd, &curr) < 0)
 		rb_sys_fail("timerfd_gettime");
 
 	return itimerspec2ary(&curr);
@@ -126,7 +126,7 @@ static VALUE expirations(int argc, VALUE *argv, VALUE self)
 		blocking_io_prepare(fd);
 retry:
 	r = (ssize_t)rb_sp_fd_region(tfd_read, &buf, fd);
-	if (r == -1) {
+	if (r < 0) {
 		if (errno == EAGAIN && RTEST(nonblock))
 			return Qnil;
 		if (rb_sp_wait(rb_io_wait_readable, self, &fd))
diff --git a/ext/sleepy_penguin/util.c b/ext/sleepy_penguin/util.c
index 2423af6..11a1b13 100644
--- a/ext/sleepy_penguin/util.c
+++ b/ext/sleepy_penguin/util.c
@@ -131,7 +131,7 @@ void rb_sp_set_nonblock(int fd)
 	 * anything.
 	 */
 	flags = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
-	if (flags == -1)
+	if (flags < 0)
 		rb_sys_fail("fcntl(F_SETFL)");
 }
 
-- 
Eric Wong


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2013-04-21  3:09 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-21  3:09 [sleepy.penguin] [PATCH] favor comparison against 0 for error checking Eric Wong

Code repositories for project(s) associated with this public inbox

	https://yhbt.net/sleepy_penguin.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).