Closed cglogic closed 2 years ago
Sounds reasonable to me. Yeah, the current design has that problem with structs that contain members called close
, read
etc.
So the downside with function like macros is that this code no longer works for "epoll-like" fds, right?
#define close(fd) epoll_shim_close(fd)
int (*close_fp)(int) = &close;
close_fp(some_fd);
I am not aware of any consumers of epoll-shim using that pattern, but now I am aware of consumers using .close
/.read
in designated initializers.
I really don't want that valid code written for Linux that uses epoll
needs any source changes to work with epoll-shim. The goal should be that most code "just works" when ported over to *BSD. Of course that won't always work perfectly, but I think we can get pretty close.
So the downside with function like macros is that this code no longer works for "epoll-like" fds, right?
That would be correct.
I assume you have read the issue linked by @cglogic (since your example looks like it was copied from there)?
I just wanted to push the fact that we (foot) have worked around this and by no means depend on https://github.com/jiixyj/epoll-shim/pull/31. It would be a nice-to-have, but I have no idea whether it would break other applications :)
If there are no known consumers that work with pointers to functions this can help in porting software like foot.
I'm not sure what is the best solution here.
Another fun thing I noticed while playing around with this is that C's function like macros don't interact well with compound literals, so something like this errors out: ppoll(&pfd, 1, &(struct timespec) { 0, 0 }, &emptyset)
I'm now looking at defining the wrapper macros like this: #define ppoll(...) epoll_shim_ppoll(__VA_ARGS__)
. This seems to do the trick.
I can think of two more problems with that approach, but luckily there are workarounds:
Actually calling a function pointer that has a conflicting name. You can fix this by wrapping the reference to the function pointer in parentheses:
int fd;
struct conflict conflict = {
.close = conflict_close,
.read = conflict_read,
};
(conflict.close)(fd);
Overriding standard library functions that have a conflicting name. For example, you can replace the global read
function like this:
#define NO_MACRO_EXPAND /**/
ssize_t read NO_MACRO_EXPAND(int fd, void *buf, size_t count) { ... }
The preprocessor will detect another token between the `read` and the `(` and thus won't expand the function-like `read` macro.
Another fun thing I noticed while playing around with this is that C's function like macros don't interact well with compound literals, so something like this errors out:
ppoll(&pfd, 1, &(struct timespec) { 0, 0 }, &emptyset)
I'm now looking at defining the wrapper macros like this:
#define ppoll(...) epoll_shim_ppoll(__VA_ARGS__)
. This seems to do the trick.
Nice catch. Dealing with all cases is a bit tricky.
Found that #31 break building multimedia/v4l-utils on FreeBSD.
Is it because of std::istream
s .close()
member? What a bummer...
Luckily, such usages of .close()
can be relatively easily worked around with some automated patching in the BSD ports file like this:
echo " stream.close()" | sed -e 's|\([^[:space:]]*\.close\)(|(\1)(|g'
In addition, I guess epoll-shim
should give porters and "epoll-shim
aware" consumers some means of controlling if/when those wrapper macros are defined, because it's not possible to cover all cases as we've seen.
For example, in this case here, the close(
wrapper macro would stomp over the internal close
usages in <fstream>
:
#include <sys/epoll.h>
#include <fstream>
#include <iostream>
int main() {
std::ifstream f;
f.close();
int ep = epoll_create1(0);
close(ep);
}
We could have some way of deferring the definition of the wrapper macros, for example like this:
#define EPOLL_SHIM_NO_WRAPPER_MACROS
#include <sys/epoll.h>
#include <fstream>
#include <iostream>
#include <epoll-shim/wrapper-macros.h>
int main() {
std::ifstream f;
(f.close)();
int ep = epoll_create1(0);
close(ep);
}
Even the insertion of the #define EPOLL_SHIM_NO_WRAPPER_MACROS
and #include <epoll-shim/wrapper-macros.h>
lines could be automated with some kind of sed
magic in the ports file, so no code changes would be needed:
#define EPOLL_SHIM_NO_WRAPPER_MACROS
could be the first line of the file#include <epoll-shim/wrapper-macros.h>
would be the first line after the last #include
line of the original fileSorry for the delay.
After adding parameters for the macro close
there is a situation where declaration of pointer to a function named close
not touched but calls of close
changed to epoll_shim_close
.
The error:
In file included from ../../utils/common/cv4l-helpers.h:13:
../../utils/common/v4l-helpers.h:576:5: error: no member named 'epoll_shim_close' in 'v4l_fd'
f->close(f);
~ ^
/usr/local/include/libepoll-shim/epoll-shim/detail/common.h:8:20: note: expanded from macro 'close'
#define close(...) epoll_shim_close(__VA_ARGS__)
v4l-helpers.h
contains:
struct v4l_fd {
...
int (*close)(struct v4l_fd *f);
...
};
For corner cases like the above, the library now provides read
/write
/close
/poll
/ppoll
/fcntl
symbols in libepoll-shim-interpose.so.0
. There is a new pkg-config
file epoll-shim-interpose.pc
for this mode. When using this, no macro redefinitions take place which should be much more robust.
Closing this for now.
Defining
close
and friends without arguments has border cases with include order.Maybe it has a sense to change the definitions from
#define close epoll_shim_close
to#define close(fd) epoll_shim_close(fd)
? But this change breaks function pointer assignments. What do you think? I can create pull request this change has sense for you.Please see for details https://codeberg.org/dnkl/foot/issues/500#issuecomment-197764