Skip to content

Commit e683648

Browse files
committed
Workaround in poll_set_fd for STDIN_FILENO: avoid returning EPERM (it can be actually polled). Hopefully fixed tests on osx.
1 parent d2f667b commit e683648

File tree

4 files changed

+34
-17
lines changed

4 files changed

+34
-17
lines changed

Lib/module.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "poll_priv.h"
33
#include <string.h>
44
#include <stdarg.h>
5+
#include <errno.h>
56

67
static module_ret_code init_ctx(const char *ctx_name, m_context **context);
78
static void destroy_ctx(const char *ctx_name, m_context *context);
@@ -12,6 +13,7 @@ static int tell_if(void *data, void *m);
1213
static pubsub_msg_t *create_pubsub_msg(const unsigned char *message, const self_t *sender, const char *topic, enum msg_type type, const size_t size);
1314
static module_ret_code tell_pubsub_msg(pubsub_msg_t *m, module *mod, m_context *c);
1415
static int manage_fds(module *mod, m_context *c, int flag, int stop);
16+
static int _poll_set_new_evt(module_poll_t *t, m_context *c, int flag);
1517
static module_ret_code start(const self_t *self, const enum module_states mask, const char *err_str);
1618
static module_ret_code stop(const self_t *self, const enum module_states mask, const char *err_str, int stop);
1719

@@ -196,7 +198,7 @@ module_ret_code module_register_fd(const self_t *self, const int fd, const bool
196198

197199
/* If a fd is registered at runtime, start polling on it */
198200
if (module_is(self, RUNNING)) {
199-
int ret = poll_set_new_evt(tmp, c, ADD);
201+
int ret = _poll_set_new_evt(tmp, c, ADD);
200202
return !ret ? MOD_OK : MOD_ERR;
201203
}
202204
return MOD_OK;
@@ -216,7 +218,7 @@ module_ret_code module_deregister_fd(const self_t *self, const int fd) {
216218
*tmp = (*tmp)->prev;
217219
/* If a fd is deregistered for a RUNNING module, stop polling on it */
218220
if (module_is(self, RUNNING)) {
219-
ret = poll_set_new_evt(t, c, RM);
221+
ret = _poll_set_new_evt(t, c, RM);
220222
}
221223
if (t->autoclose) {
222224
close(t->fd);
@@ -445,12 +447,25 @@ static int manage_fds(module *mod, m_context *c, int flag, int stop) {
445447
if (flag == RM && stop) {
446448
ret = module_deregister_fd(&mod->self, t->fd);
447449
} else {
448-
ret = poll_set_new_evt(t, c, flag);
450+
ret = _poll_set_new_evt(t, c, flag);
449451
}
450452
}
451453
return ret;
452454
}
453455

456+
/*
457+
* Just a call to poll_set_new_evt + some filtering:
458+
* STDIN_FILENO returns EPERM but it is actually pollable
459+
*/
460+
static int _poll_set_new_evt(module_poll_t *t, m_context *c, int flag) {
461+
int ret = poll_set_new_evt(t, c, flag);
462+
/* Workaround for STDIN_FILENO */
463+
if (ret == -1 && t->fd == STDIN_FILENO && errno == EPERM) {
464+
ret = 0;
465+
}
466+
return ret;
467+
}
468+
454469
static module_ret_code start(const self_t *self, const enum module_states mask, const char *err_str) {
455470
GET_MOD_IN_STATE(self, mask);
456471

Lib/poll_plugins/kqueue_priv.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ int poll_create(void) {
77
int fd;
88
#ifdef __APPLE__
99
fd = kqueue();
10-
#ifdef O_CLOEXEC
10+
// #ifdef FD_CLOEXEC
1111
fcntl(fd, F_SETFD, FD_CLOEXEC);
12-
#endif
12+
// #endif
1313
#else
1414
fd = kqueue1(O_CLOEXEC);
1515
#endif
@@ -68,9 +68,9 @@ int _pipe(int fd[2]) {
6868
flags = 0;
6969
}
7070
fcntl(fd[i], F_SETFL, flags | O_NONBLOCK);
71-
#ifdef O_CLOEXEC
71+
// #ifdef FD_CLOEXEC
7272
fcntl(fd[i], F_SETFD, FD_CLOEXEC);
73-
#endif
73+
// #endif
7474
}
7575
}
7676
return ret;

TODO.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
- [x] module_stop can be called on PAUSED modules too!
77
- [x] Document module states! (STOP is same as IDLE; IDLE means it is registered but still never started. STOPPED it means it is registered, was started and was stopped.)
88

9-
- [x] Set O_CLOEXEC flag in non-linux poll implementation for both pipe and kqueue
9+
- [x] Set FD_CLOEXEC flag in non-linux poll implementation for both pipe and kqueue
10+
11+
- [x] Fix CMakeLists to runt tests even if valgrind could not be found
12+
13+
- [ ] Fix tests on osx? module_(de)register_fd fails
1014

1115
## 3.1.0
1216

tests/test_module.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ static void recv(const msg_t *msg, const void *userdata);
1212
static void destroy(void);
1313

1414
const self_t *self = NULL;
15-
int fd;
1615

1716
void test_module_register_NULL_name(void **state) {
1817
(void) state; /* unused */
@@ -212,22 +211,21 @@ void test_module_become(void **state) {
212211
void test_module_add_wrong_fd(void **state) {
213212
(void) state; /* unused */
214213

215-
module_ret_code ret = module_register_fd(self, -1, 1, NULL);
214+
module_ret_code ret = module_register_fd(self, -1, true, NULL);
216215
assert_false(ret == MOD_OK);
217216
}
218217

219218
void test_module_add_fd_NULL_self(void **state) {
220219
(void) state; /* unused */
221220

222-
fd = open("/dev/tty", O_RDWR);
223-
module_ret_code ret = module_register_fd(NULL, fd, 1, NULL);
221+
module_ret_code ret = module_register_fd(NULL, STDIN_FILENO, true, NULL);
224222
assert_false(ret == MOD_OK);
225223
}
226224

227225
void test_module_add_fd(void **state) {
228226
(void) state; /* unused */
229227

230-
module_ret_code ret = module_register_fd(self, fd, 1, NULL);
228+
module_ret_code ret = module_register_fd(self, STDIN_FILENO, true, NULL);
231229
assert_true(ret == MOD_OK);
232230
}
233231

@@ -241,14 +239,14 @@ void test_module_rm_wrong_fd(void **state) {
241239
void test_module_rm_wrong_fd_2(void **state) {
242240
(void) state; /* unused */
243241

244-
module_ret_code ret = module_deregister_fd(self, fd + 1);
242+
module_ret_code ret = module_deregister_fd(self, STDIN_FILENO + 1);
245243
assert_false(ret == MOD_OK);
246244
}
247245

248246
void test_module_rm_fd_NULL_self(void **state) {
249247
(void) state; /* unused */
250248

251-
module_ret_code ret = module_deregister_fd(NULL, fd);
249+
module_ret_code ret = module_deregister_fd(NULL, STDIN_FILENO);
252250
assert_false(ret == MOD_OK);
253251
}
254252

@@ -259,11 +257,11 @@ static int fd_is_valid(int fd) {
259257
void test_module_rm_fd(void **state) {
260258
(void) state; /* unused */
261259

262-
module_ret_code ret = module_deregister_fd(self, fd);
260+
module_ret_code ret = module_deregister_fd(self, STDIN_FILENO);
263261
assert_true(ret == MOD_OK);
264262

265263
/* Fd is now closed (module_deregister_fd with 1 flag) thus is no more valid */
266-
assert_false(fd_is_valid(fd));
264+
assert_false(fd_is_valid(STDIN_FILENO));
267265
}
268266

269267
void test_module_subscribe_NULL_topic(void **state) {

0 commit comments

Comments
 (0)