Skip to content

gpg: set default in function to allow global patch#543

Merged
lukpueh merged 1 commit intosecure-systems-lab:mainfrom
lukpueh:allow-patch-timeout
Mar 21, 2023
Merged

gpg: set default in function to allow global patch#543
lukpueh merged 1 commit intosecure-systems-lab:mainfrom
lukpueh:allow-patch-timeout

Conversation

@lukpueh
Copy link
Copy Markdown
Member

@lukpueh lukpueh commented Mar 21, 2023

Patching globals to modify function behavior is an anti-pattern and should not be recommended to a user!!

But it seems acceptable in tests (see #542). For this to work, the global must not be used before being modified, which, AFAIK, is not possible, if the global is defined and used (e.g. in a function signature) in the same scope.

This commit changes the relevant function to access the default value in local scope, so that it global can be imported and modified before it.

More invasive but probably more correct solutions would be:

  • move either GPG_TIMEOUT or is_available_gnupg to a different module, or
  • call relevant functions in tests with an explicit timeout arg

Patching globals to modify function behavior is an anti-pattern and
should not be recommended to a user!!

But it seems acceptable in tests (see secure-systems-lab#542). For this to work, the
global must not be used before being modified, which, AFAIK, is not
possible, if the global is defined and used (e.g. in a function
signature) in the same scope.

This commit changes the relevant function to access the default
value in local scope, so that it global can be imported and
modified before it.

More invasive but probably more correct solutions would be:
- move either GPG_TIMEOUT or is_available_gnupg to a different
  module, or
- call relevant functions in tests with an explicit timeout arg

Signed-off-by: Lukas Puehringer <[email protected]>
Copy link
Copy Markdown
Collaborator

@jku jku left a comment

Choose a reason for hiding this comment

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

This seems correct even if the other improvements are later planned

@lukpueh lukpueh requested a review from jku March 21, 2023 11:38
@lukpueh lukpueh merged commit 4da1e0e into secure-systems-lab:main Mar 21, 2023
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