Skip to content

mpi_t: Remove invalid error check #7565

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

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

mpi_t: Remove invalid error check #7565

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36a4e3d
mpi_t: Remove invalid error check
raffenet Sep 10, 2025
87e7b13
mpi_t: Rename variable for clarity
raffenet Sep 18, 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
13 changes: 6 additions & 7 deletions maint/extractcvars
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ print OUTPUT_C <<EOT;
int ${fn_ns}_init(void)
{
int mpi_errno = MPI_SUCCESS;
int rc, got_rc;
int rc, is_set;
const char *tmp_str;
MPIR_T_cvar_value_t defaultval;

Expand Down Expand Up @@ -327,34 +327,33 @@ foreach my $p (@cvars) {
push @env_names, "${alt_ns}_" . $p->{name};
push @env_names, "${uc_ns}_" . $p->{name};

print OUTPUT_C " got_rc = 0;\n";
print OUTPUT_C " is_set = 0;\n";
foreach my $env_name (@env_names) {
# assumes rc is defined
if ($p->{type} eq 'range') {
print OUTPUT_C <<EOT;
rc = MPL_env2${env_fn}("$env_name", &($var_name.low), &($var_name.high));
MPIR_ERR_CHKANDJUMP1((-1 == rc),mpi_errno,MPI_ERR_OTHER,"**envvarparse","**envvarparse %s","$env_name");
got_rc += rc;
is_set += rc;
EOT
}
elsif ($p->{type} eq 'string' or $p->{type} eq 'enum') {
print OUTPUT_C <<EOT;
rc = MPL_env2${env_fn}("$env_name", &tmp_str);
MPIR_ERR_CHKANDJUMP1((-1 == rc),mpi_errno,MPI_ERR_OTHER,"**envvarparse","**envvarparse %s","$env_name");
Copy link
Contributor

@hzhou hzhou Sep 18, 2025

Choose a reason for hiding this comment

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

It doesn't hurt to leave the error check in. Even though MPL_env2str never returns error, it could in design. I think leave the error check in for uniformity is easier to maintain.

got_rc += rc;
is_set += rc;
EOT
}
else {
print OUTPUT_C <<EOT;
rc = MPL_env2${env_fn}("$env_name", &($var_name));
MPIR_ERR_CHKANDJUMP1((-1 == rc),mpi_errno,MPI_ERR_OTHER,"**envvarparse","**envvarparse %s","$env_name");
got_rc += rc;
is_set += rc;
Copy link
Contributor

@hzhou hzhou Sep 18, 2025

Choose a reason for hiding this comment

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

Not sure the renaming is any better.

I think the pr is overall a gratuitous/unnecessary change.

EOT
}
}

# debugging
print OUTPUT_C " if (got_rc && debug) {\n";
print OUTPUT_C " if (is_set && debug) {\n";
if ($p->{type} eq 'string' or $p->{type} eq 'enum') {
print OUTPUT_C " printf(\"CVAR: $var_name = %s\\n\", tmp_str);\n";
} elsif ($p->{type} eq "range") {
Expand Down