-
Notifications
You must be signed in to change notification settings - Fork 632
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
Conversation
|
This pull request introduces 1 alert when merging 1373136 into 9a81078 - view on LGTM.com new alerts:
|
1373136 to
e4f1380
Compare
kmk3
left a comment
There was a problem hiding this 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
e4f1380 to
0d06369
Compare
kmk3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
merged, thanks! |
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.
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.
Replace the hardcoded `MAX_ENVS` and `MAX_ENV_LEN` limits with new global configuration options, `env-max-count` and `env-max-len`, which limit the maximum number of environment variables and the maximum length of each environment variable (respectively). Also, include the environment name and value in the "too long environment variable" error message, similarly to the "too long argument" error message (see PR netblue30#4676 and PR netblue30#5677). This is a follow-up to netblue30#6878. Closes netblue30#3678.
Replace the hardcoded `MAX_ENVS` and `MAX_ENV_LEN` limits with new global configuration options, `env-max-count` and `env-max-len`, which limit the maximum number of environment variables and the maximum length of each environment variable (respectively). Also, include the environment name and value in the "too long environment variable" error message, similarly to the "too long argument" error message (see PR #4676 and PR #5677). This is a follow-up to #6878. Closes #3678.
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