Skip to content

Make env/arg sanity check failure messages more useful #4676

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

Merged
merged 1 commit into from
Nov 11, 2021

Conversation

hlein
Copy link
Contributor

@hlein hlein commented Nov 10, 2021

This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger [email protected]
Bug: #3678
Bug: #3851
Bug: #4633

@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2021

This pull request introduces 1 alert when merging 1373136 into 9a81078 - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@hlein hlein force-pushed the firejail_envchecks branch from 1373136 to e4f1380 Compare November 10, 2021 22:28
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

Other than the suggestions, LGTM.

This change doesn't alter any checks, but it gives more specific
errors when a sanity check of env vars or argv does not pass, which
can point to limits to raise or at least give us better detailed bug
reports.

Signed-off-by: Hank Leininger <[email protected]>
Bug: netblue30#3678
Bug: netblue30#3851
Bug: netblue30#4633
@hlein hlein force-pushed the firejail_envchecks branch from e4f1380 to 0d06369 Compare November 10, 2021 22:58
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

LGTM.

@netblue30 netblue30 merged commit 98b8172 into netblue30:master Nov 11, 2021
@netblue30
Copy link
Owner

merged, thanks!

kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 21, 2023
Also, s/arguments/argument/ since the message refers to one specific
argument.

Relates to commit 0d06369 ("Make env/arg sanity check failure messages
more useful", 2021-11-10) / PR netblue30#4676.
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 21, 2023
Also, s/arguments/argument/ since the message refers to one specific
argument.

Relates to commit 0d06369 ("Make env/arg sanity check failure messages
more useful", 2021-11-10) / PR netblue30#4676.

Relates to netblue30#5676.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants