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

Unify PAM return value usage #751

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Jan 20, 2024

PAM defines a set of common return-values, like PAM_SUCCESS or PAM_AUTH_ERR, which are used internally and in public API. Commonly these return-values are stored in variables named retval.

Unify the naming of variables such that if-and-only-if a variable is named retval it holds a PAM style return-value.

Also use explicitly PAM_SUCESS instead of the value 0 in assignments and comparisons.

PAM defines a set of common return-values, like PAM_SUCCESS or
PAM_AUTH_ERR, which are used internally and in public API.  Commonly
these return-values are stored in variables named `retval`.

Unify the naming of variables such that if-and-only-if a variable is
named `retval` it holds a PAM style return-value.

Also use explicitly PAM_SUCESS instead of the value 0 in assignments and
comparisons.
@stoeckmann
Copy link
Contributor

stoeckmann commented Jan 20, 2024

While I agree with using PAM_SUCCESS instead of 0, I'm not sure where the advantage of unification of retval is. It's not like it's any special term. Contrary, it's just a short version of "return value". How should anyone guarantee that future commits won't use "retval" as return value for something else again?

@ldv-alt
Copy link
Member

ldv-alt commented Jan 21, 2024

You could play with creating a enum, e.g. pam_retval_t, and using it consistently instead of int, although I'm not sure we can change the public API this way, even though the enum and int would have the same representation.

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

Successfully merging this pull request may close these issues.

None yet

3 participants