~emersion/basu#13: 
Namespace internal struct ucred

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

https://github.com/emersion/basu/blob/8cd22051ef8e238e3682825cc00bfd5198450076/src/basic/socket-util.h#L27

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

Thanks!

Status
REPORTED
Submitter
github.com:tuxillo (unverified)
Assigned to
No-one
Submitted
3 years ago
Updated
3 years ago
Labels
No labels applied.

github.com:kennylevinsen (unverified) 3 years ago · edit

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

github.com:emersion (unverified) 3 years ago · edit

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

github.com:tuxillo (unverified) 3 years ago · edit

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.

github.com:kennylevinsen (unverified) 3 years ago · edit

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.

Register here or Log in to comment, or comment via email.