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.

Sign up for GitHub

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

New Get_ENV Functions #1141

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

New Get_ENV Functions #1141

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6398563
check env null
waahm7 Jul 31, 2024
70d97a8
fix lint
waahm7 Jul 31, 2024
296de34
wip impl
waahm7 Aug 5, 2024
ffa0ae6
Merge branch 'main' into fix-env-variable-fucntion-2
waahm7 Aug 5, 2024
aa565ee
lint
waahm7 Aug 5, 2024
4d2fb12
skip empty comparison on Windows
waahm7 Aug 5, 2024
e5e7776
fix warning windows
waahm7 Aug 5, 2024
1d15439
Merge branch 'main' into fix-env-variable-fucntion-2
waahm7 Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/aws/common/environment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ struct aws_string;
AWS_EXTERN_C_BEGIN

/*
* Get the value of an environment variable. If the variable is not set, the output string will be set to NULL.
* Not thread-safe
* Get the value of an environment variable. If the variable is not set or is empty, the output string will be set to
* NULL. Not thread-safe
*/
AWS_COMMON_API
int aws_get_environment_value(
Expand Down
2 changes: 1 addition & 1 deletion source/posix/environment.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ int aws_get_environment_value(
struct aws_string **value_out) {

const char *value = getenv(aws_string_c_str(variable_name));
if (value == NULL) {
if (value == NULL || value[0] == '\0') {
Copy link
Contributor

Choose a reason for hiding this comment

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

It sucks, but this is how it is with environment variables. Sometimes just the existence of the env-var is meaningful, even if it's blank. Maybe create an alternate function like aws_get_environment_variable_nonempty() that has this behavior, and scrub the codebase for places that would benefit from using that instead

Copy link
Contributor Author

@waahm7 waahm7 Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I agree. We can discuss the following options in the office tomorrow.

  1. Modify the existing function, and we can create a new function in the future if required, like if we have a use case where env-var can be blank.

  2. Create a new function, aws_get_environment_variable_nonempty, aws_get_environment_variable_cursor, or aws_get_environment_variable_c_str. I prefer the aws_get_environment_variable_cursor if we decide to just create a new function.

  3. Keep the function as it is, and validate all the uses to handle it correctly.

*value_out = NULL;
return AWS_OP_SUCCESS;
}
Expand Down
2 changes: 1 addition & 1 deletion source/windows/environment.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ int aws_get_environment_value(
# pragma warning(pop)
#endif

if (value == NULL) {
if (value == NULL || value[0] == '\0') {
*value_out = NULL;
return AWS_OP_SUCCESS;
}
Expand Down
Loading