Skip to content

Macros, Conv: Handle edge cases, refactor macros#51

Merged
lvkv merged 3 commits into
mainfrom
lvkv/fixes-11
May 28, 2026
Merged

Macros, Conv: Handle edge cases, refactor macros#51
lvkv merged 3 commits into
mainfrom
lvkv/fixes-11

Conversation

@lvkv
Copy link
Copy Markdown
Owner

@lvkv lvkv commented May 28, 2026

Three commits:

  1. A macro refactor to move as much logic out of the pam_hooks macro and into "normal" functions that can be tested more easily
  2. A fix for Conv::send so that it rejects unsupported PAM message styles; also removes support for the binary message style because the send internals assume strings, not arbitrary bytes.
  3. Handle the edge case of the PAM handle pointer itself being null

@lvkv lvkv requested a review from Copilot May 28, 2026 20:23
@lvkv lvkv self-assigned this May 28, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the PAM hook entrypoint macro to centralize safety/validation logic in testable functions, and hardens the conversation (Conv) API by rejecting unsupported PAM message styles (including binary prompts) while also handling a null PAM handle pointer edge case.

Changes:

  • Refactor pam_hooks! to call a shared invoke_hook helper that validates pamh/argc/argv and panic-guards hook execution.
  • Update Conv::send to reject unsupported/unknown message styles and drop binary prompt support.
  • Make PAM_BINARY_PROMPT non-public (crate-only) since it’s not supported by send.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pam/src/macros.rs Moves hook invocation/validation into invoke_hook, updates hook entrypoints to accept raw pamh pointer, adds/updates unit tests.
pam/src/conv.rs Adds message-style validation to Conv::send and introduces tests for rejected styles and invalid input.
pam/src/constants.rs Restricts PAM_BINARY_PROMPT visibility to pub(crate) to reflect unsupported status.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pam/src/conv.rs Outdated
Comment thread pam/src/constants.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@lvkv lvkv merged commit 763b568 into main May 28, 2026
7 checks passed
@lvkv lvkv deleted the lvkv/fixes-11 branch May 28, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants