From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS47066 71.19.144.0/20 X-Spam-Status: No, score=-1.9 required=3.0 tests=AWL,BAYES_00 shortcircuit=no autolearn=unavailable version=3.3.2 X-Original-To: normalperson@yhbt.net Received: from zedshaw2.xen.prgmr.com (zedshaw2.xen.prgmr.com [71.19.156.177]) by dcvr.yhbt.net (Postfix) with ESMTP id A5BBC1F5B5 for ; Sun, 21 Apr 2013 03:09:56 +0000 (UTC) Received: from zedshaw2.xen.prgmr.com (unknown [IPv6:::1]) by zedshaw2.xen.prgmr.com (Postfix) with ESMTP id 6D49473DFC for ; Sun, 21 Apr 2013 03:11:12 +0000 (UTC) MIME-Version: 1.0 Date: Sun, 21 Apr 2013 03:09:42 +0000 From: Eric Wong In-Reply-To: <20130421030942.GA24421@dcvr.yhbt.net> List-Archive: List-Help: List-Id: List-Post: List-Subscribe: List-Unsubscribe: Message-Id: <20130421030942.GA24421@dcvr.yhbt.net> Precedence: list References: <20130421030942.GA24421@dcvr.yhbt.net> Sender: sleepy.penguin@librelist.org Subject: [sleepy.penguin] [PATCH] favor comparison against 0 for error checking To: sleepy.penguin@librelist.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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