From: Eric Wong <normalperson@yhbt.net>
To: sleepy.penguin@librelist.org
Subject: [sleepy.penguin] [PATCH] favor comparison against 0 for error checking
Date: Sun, 21 Apr 2013 03:09:42 +0000 [thread overview]
Message-ID: <20130421030942.GA24421@dcvr.yhbt.net> (raw)
In-Reply-To: <20130421030942.GA24421@dcvr.yhbt.net>
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
parent reply other threads:[~2013-04-21 3:09 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20130421030942.GA24421@dcvr.yhbt.net>]
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/sleepy_penguin/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130421030942.GA24421@dcvr.yhbt.net \
--to=normalperson@yhbt.net \
--cc=sleepy.penguin@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/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).