flashmq_poll_remove_fd incorrect descriptor type

[slavslavov]

In flashmq_plugin.h, the argument of flashmq_poll_remove_fd has type uint32_t while in the other two flashmq_poll_XX functions it is int

[halfgaar]

Hmm. Changing this could theoretically break the ABI. Currently it does not because uint32_t is unsigned int on all platforms, but it's still a bit of risk should the size change.

It would require something like an extra definition in an inline namespace:

diff --git a/flashmq_plugin.cpp b/flashmq_plugin.cpp
index 4b7cae9..53b9155 100644
--- a/flashmq_plugin.cpp
+++ b/flashmq_plugin.cpp
@@ -170,6 +170,19 @@ void flashmq_poll_remove_fd(uint32_t fd)
     d->pollExternalRemove(fd);
 }

+inline namespace fmq_abi_rev_1
+{
+void flashmq_poll_remove_fd(int fd)
+{
+    ThreadData *d = ThreadGlobals::getThreadData();
+
+    if (!d)
+        return;
+
+    d->pollExternalRemove(fd);
+}
+}
+
 sockaddr *FlashMQSockAddr::getAddr()
 {
     return reinterpret_cast<struct sockaddr*>(&this->addr_in6);
diff --git a/flashmq_plugin.h b/flashmq_plugin.h
index 56bbb92..dac1e93 100644
--- a/flashmq_plugin.h
+++ b/flashmq_plugin.h
@@ -231,7 +231,10 @@ void flashmq_poll_add_fd(int fd, uint32_t events, const std::weak_ptr<void> &p);
  *
  * [Function provided by FlashMQ]
  */
-void flashmq_poll_remove_fd(uint32_t fd);
+inline namespace fmq_abi_rev_1
+{
+void flashmq_poll_remove_fd(int fd);
+}

 /**
  * @brief Is called when the socket watched by 'flashmq_poll_add_fd()' has an event.

This provides an extra, mangled, linker name _Z22flashmq_poll_remove_fdj, used by plugins built with the new header file.

I'm not sure yet if there are implications, or whether the new namespace name should actually be unique to the function name. Thinking...

[slavslavov]

It should be OK to use uint32_t for the moment. Just pointed it out if you make some interface changes in the future to have it in mind. All "good" file descriptors are positive numbers and there won't be a case to pass a negative value. Size should be OK too, at least on systems able to run Linux.

[wiebeytec]

My inline namespace trick won't work, for various reasons. The above way is wrong anyway.

It's merely a size issue BTW. If you pass it -1, it will be some huge number, but the bits are the same, and will be -1 again when passed to the next internal function, which does properly do int.

I think I can actually just change it to int. That would keep it functional for the next hypothetical platform where int is 64 bit...