Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Add eventfd bindings#1800

Merged
dlang-bot merged 1 commit intodlang:masterfrom
nemanja-boric-sociomantic:eventfd
Apr 6, 2017
Merged

Add eventfd bindings#1800
dlang-bot merged 1 commit intodlang:masterfrom
nemanja-boric-sociomantic:eventfd

Conversation

@nemanja-boric-sociomantic
Copy link
Contributor

This adds bindings for linux-specific eventfd_* calls.

version (linux):
extern (C):
@system:
nothrow:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nogc:

int eventfd (uint count, int flags);

/* Read event counter and possibly wait for events. */
int eventfd_read (int fd, uint* value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you get those signatures from?
According to https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sys/eventfd.h;hb=HEAD they should be:

/* Type for event counter.  */
typedef uint64_t eventfd_t;

/* Return file descriptor for generic event channel.  Set initial
   value to COUNT.  */
extern int eventfd (unsigned int __count, int __flags) __THROW;

/* Read event counter and possibly wait for events.  */
extern int eventfd_read (int __fd, eventfd_t *__value);

/* Increment event counter.  */
extern int eventfd_write (int __fd, eventfd_t __value);

I suggest you add and use the eventfd_t typedef accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I was wondering - should I add this typedef. I'll update.

This adds bindings for linux-specific eventfd_* calls.
@nemanja-boric-sociomantic
Copy link
Contributor Author

Updated with nogc and eventfd_t being alias to uint64_t. @ZombineDev is alias enough, or should I go with a full-blown typedef?

Copy link
Member

@PetarKirov PetarKirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PetarKirov
Copy link
Member

@nemanja-boric-sociomantic alias is more than enough - D2 doesn't have typedef as a language feature and Typedef(T) is part of Phobos, so it can't be used in druntime.

@nemanja-boric-sociomantic
Copy link
Contributor Author

Yeah, that's what I thought, but simple grep found some Typedefs in some runtime modules:

nemanjaboric@labs-129:/home/nemanjaboric/work/druntime  git:(eventfd*) $ ag -l TypeDef .               
src/core/sys/windows/ole.d
src/core/sys/windows/mmsystem.d
src/core/sys/windows/rpcdcep.d
src/core/sys/windows/basetsd.d
src/core/sys/windows/vfw.d
src/core/sys/windows/sqltypes.d
src/core/sys/windows/ras.d
src/core/sys/windows/rpcndr.d
src/core/sys/windows/winuser.d
src/core/sys/windows/wingdi.d

@PetarKirov
Copy link
Member

AFAICS, this was something that @CyberShadow did and is used only for the Win32 bindings. My understanding of this is that the Win32 bindings were originally written in D1 and were later ported to D2 while keeping D1 backwards compatibility and now this is no longer needed. Also none of the other system bindings (stdc/, sys/) use this technique. @CyberShadow can you shed some light on this?

@CyberShadow
Copy link
Member

That TypeDef definition was from back when the win32 package was still only available separately, on DSource. At some point typedef was a proper language feature in D2, however it became deprecated (in favor of the Phobos definition) and later removed, thus that change.

Although strong typing for the various integral (or void*-aliased) Windows types would be nice, it is far too late to change it now, and would make porting code from C/C++ more onerous. So, the use and definition of TypeDef in the core.sys.windows package can now be removed.

@PetarKirov
Copy link
Member

Thanks for the explanation @CyberShadow. Are you OK with merging this PR?

@CyberShadow
Copy link
Member

What's the reason for the large version switch? Looking at the Linux source code, these don't seem to be architecture-dependent constants.

@CyberShadow
Copy link
Member

Huh, so they are in glibc. Not in the Linux kernel though, as far as I could tell.

@nemanja-boric-sociomantic
Copy link
Contributor Author

Yeah, I'll give it another look to be 100% sure.

@CyberShadow
Copy link
Member

Well there's no harm in doing it the glibc way, so LGTM.

@nemanja-boric-sociomantic
Copy link
Contributor Author

Thanks!

@dlang-bot dlang-bot merged commit fcbec36 into dlang:master Apr 6, 2017
@nemanja-boric-sociomantic nemanja-boric-sociomantic deleted the eventfd branch April 6, 2017 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants