Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

Namespace internal struct ucred #34

Open
tuxillo opened this issue Feb 14, 2021 · 4 comments
Open

Namespace internal struct ucred #34

tuxillo opened this issue Feb 14, 2021 · 4 comments

Comments

@tuxillo
Copy link

tuxillo commented Feb 14, 2021

It looks to me as if struct ucred is for internal basu usage (I might be wrong):

struct ucred {

If that's the case, can it be renamed to struct basu_ucred ?

Thanks!

@kennylevinsen
Copy link
Collaborator

I assume you're experiencing issues as a result? If so, please describe these. Otherwise this just seems like a style change suggestion.

A PR that renames the internal struct ucred to struct basu_cred would be accepted, which implies writing conversion functions to/from struct ucred and struct xucred and changing all use to struct basu_cred

@emersion
Copy link
Owner

This isn't a public header anyways. No need to use prefixes if we just use it internally.

@tuxillo
Copy link
Author

tuxillo commented Feb 14, 2021

In FreeBSD, struct ucred is under _KERNEL or WANT_UCRED so there is no collision. In DragonFly BSD that is not the case, hence there is a redefinition error. Other BSDs might have the same issue but I have not checked.

Even tho the structure is internal, it can potentially collide with definitions from sys/ucred.h as far as I can tell.

@kennylevinsen
Copy link
Collaborator

Options:

  1. Gate struct ucred better - if DragonFly BSD already has it, and it is compatible, there is no need to define it.
  2. Make FreeBSD expose struct ucred - We still need to map to/from xucred, as that's what the ioctls use, but there's no need to define it then.
  3. Make our own internal credential structure and map on all OS's regardless of native structure.

The current struct ucred definition was made to allow drop-in support for FreeBSD in the UNIX peer credentials code paths, as the structure was used throughout the codebase. I don't think there's that many uses left though, so it wouldn't be that big a patch to change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants