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.

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

Use of uninitialized values in GenericMoltresMaterial #75

Closed
lindsayad opened this issue Sep 26, 2018 · 5 comments
Closed

Use of uninitialized values in GenericMoltresMaterial #75

lindsayad opened this issue Sep 26, 2018 · 5 comments

Comments

@lindsayad
Copy link
Member

@gridley, git blame tells me that this is your code in GenericMoltresMaterial::monotoneCubicConstruct (and also similarly in GenericMoltresMaterial::linearConstruct):

  Real value;
  int tempLength;
  bool onewarn = false;
  for (decltype(xsec_names.size()) j = 0; j < xsec_names.size(); ++j)
  {
    std::vector<Real> temperature;
    std::string file_name = property_tables_root + _file_map[xsec_names[j]] + ".txt";
    const std::string & file_name_ref = file_name;
    std::ifstream myfile(file_name_ref.c_str());
    auto o = _vec_lengths[xsec_names[j]];
    std::map<std::string, std::vector<std::vector<Real>>> xsec_map;
    xsec_map[xsec_names[j]].resize(o);
    _xsec_monotone_cubic_interpolators[xsec_names[j]].resize(o);

    //chi_d backwards compatibility on unit tests:
    if (xsec_names[j]=="CHI_D" and not myfile.good())
    {
      for (decltype(o) k = 0; k < o; ++k)
        for (int i = 0; i < tempLength; ++i)

In that last line, tempLength is used uninitialized. What should its value be?

@katyhuff
Copy link
Member

@gridley Perhaps it should be the length of the temperature vector initialized above?

@gridley
Copy link
Collaborator

gridley commented Sep 26, 2018

Yep! Although there's no stuff in the temperature vector at that point. I'm really confused how this could have got there. Probably some accidental line deletion which didn't get caught in units tests, since this is in a code section which supports not using CHI_D, a previously missing feature in the code. If CHI_D is supplied, this shouldn't get called. Anyways... yes. I'm going to fix this now.

@gridley
Copy link
Collaborator

gridley commented Sep 26, 2018

It may appear to not be initialized, but always should be by the time that for (j=0; j<xsec_names.size;++j) loop gets to it. That's because xsec_names is hard-coded, and CHI_D always comes past the first element. As long as the first element has been passed, tempLength should be defined.

I think it makes sense to write the code this way. This part of code only gets turned on if CHI_D is missing, and you have to assume that the list of temperatures given for CHI_D matches the length of what was given in previous XSEC files. The best way to remove the "possibility" (someone editing the xsec_names variable) would be assume that cross sections can be interpolated in disparate ways, e.g. cubic splines on CHI, monotone cubics on CHI_D. Then CHI_D could be given a constant value of 1 in group 1. Since that would be a bit of effort and of marginal benefit, I propose we leave the code as-is. I hope this explanation doesn't suck.

@lindsayad
Copy link
Member Author

You do assignment in splineConstruct but you don't in the methods I cited. In all three cases, you might as well zero initialize tempLength upon declaration to silence the compiler warning. Otherwise you are going to create concerned users when they see compiler warnings

@katyhuff
Copy link
Member

Closed with #76

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

No branches or pull requests

3 participants