Define read, write, close, poll and ppoll with parameters

[cglogic]

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

[jiixyj]

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.

[dnkl]

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 #31. It would be a nice-to-have, but I have no idea whether it would break other applications :)

[cglogic]

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.

[jiixyj]

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:

1. 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);

2. 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.

[cglogic]

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.

[cglogic]

Found that #31 break building multimedia/v4l-utils on FreeBSD.

[jiixyj]

Is it because of std::istreams .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 file

[cglogic]

Sorry 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);
   ...
};

[jiixyj]

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.