Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

define pid_t for different platforms and use it for filedescriptors #480

Closed
dkroenke opened this issue Jan 5, 2021 · 4 comments
Closed
Assignees
Labels
globex refactoring Refactor code without adding features
Milestone

Comments

@dkroenke
Copy link
Member

dkroenke commented Jan 5, 2021

Brief feature description

In iceoryx we are creating and using often filedescriptors. In POSIX standard the type for descriptors is defined as pid_t in <sys/types.h> on Unix.
In iceoryx we have iceoryx_utils/platform/types.hpp for the diferent platforms as header available. This should be consistently used instead of int for all filedescriptors in iceoryx.
Please be aware that windows doesn't know pid_t, it make sense to add a define for having it as int.

@dkroenke dkroenke added the refactoring Refactor code without adding features label Jan 5, 2021
@dkroenke dkroenke added this to the Prio 3 milestone Jan 5, 2021
@elfenpiff
Copy link
Contributor

elfenpiff commented Jan 12, 2021

@dkroenke I would recommend to introduce iox_pid_t which is an alias for pid_t in linux/mac/qnx and something else in windows. I already did it with the semaphores, see: https://github.com/eclipse-iceoryx/iceoryx/blob/master/iceoryx_utils/platform/linux/include/iceoryx_utils/platform/semaphore.hpp
where I introduced iox_sem_t in the same manner.

@dkroenke
Copy link
Member Author

@elfenpiff I'm not sure about the naming convention. I understand that for the semaphore we add the "iox_" prefix because we made a custom implementation for it on the different platforms.
But for a simple datatype like pid_t from <sys/types.h> i see no reason to add an iox_ prefix here because we do not make a custom implementation of it.

@elfenpiff
Copy link
Contributor

@dkroenke I hope you are right but sometimes Windows suprises you and then we have another iox_ prefix ... But for now you are right, we should not over complicate things!

@elBoberido
Copy link
Member

Closing in favor of #2289 which shall add the prefix to all entities in the platform abstraction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
globex refactoring Refactor code without adding features
Projects
Status: Done
Development

No branches or pull requests

3 participants