Skip to content
Snippets Groups Projects

Feature/reduce warnings clang13

Merged Feature/reduce warnings clang13
feature/reduce-warnings-clang13 into master
1 unresolved thread
Merged Christoph Grüninger requested to merge feature/reduce-warnings-clang13 into master
1 unresolved thread

Some more warnings found by Clang 13

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
dune/uggrid/gm/gmcheck.cc
1144 1144 INT side,found,i,j,k,nerrors,sdid,sc;
1145 1145 NODE *theNode,*n1,*n2,*nbn1,*nbn2,*nbn3,*nbn4;
1146 1146 EDGE *theEdge,*father_edge;
1147 ELEMENT *theFather;
1147 ELEMENT *theFather = EFATHER(theElement);
  • Oliver Sander
    Oliver Sander @oliver.sander ·
    Owner

    This is not correct. theElement may not actually have a father, and then later code may crash.

    OTOH the current code is broken, too: theFather never gets initialized. Which makes me wonder if that code is ever called. Can you write a unit test test triggers this code path?

  • Christoph Grüninger changed this line in version 2 of the diff

    changed this line in version 2 of the diff

  • Christoph Grüninger
    Christoph Grüninger @gruenich ·
    Author Owner

    Thanks for your comment, in principle you are right. I altered the position of the initialization of theFather, it is now right after the NULL check and is fine for sure. I have to check whether the code is ever executed.

    Edited by Christoph Grüninger
  • Christoph Grüninger
    Christoph Grüninger @gruenich ·
    Author Owner

    Oliver, you are right. Within dune-uggrid, above code in CheckElementSubdomains is only called by CheckSubdomains. The latter is never called.

  • Christoph Grüninger
    Christoph Grüninger @gruenich ·
    Author Owner

    The code has not a single line of comment what the methods do and they are currently unused. Oliver, would it be ok if I would simply remove it?

  • Oliver Sander
    Oliver Sander @oliver.sander ·
    Owner

    No. This is some sort of unit test, and I'd rather not delete it simply because none of us has taken the time to figure out what it doesn.

  • Christoph Grüninger
    Christoph Grüninger @gruenich ·
    Author Owner

    I see. All we have to do is adding a main function, creating a GRID and calling CheckGrid. Then all checks are run. Unfortunatley, I don't know how to crate a UG grid outside of Dune. If you want, I can create an issue for this.

    With my update, this MR should be fine either way. Do you mind merging?

  • Oliver Sander
    Oliver Sander @oliver.sander ·
    Owner

    I don't know how to crate a UG grid outside of Dune.

    Why do it outside of Dune? My idea would be to call the method from the tests in dune-grid.

    With my update, this MR should be fine either way. Do you mind merging?

    Yes, I do mind. Please revert gmcheck.cc for the time being, and I'll merge the rest.

  • Please register or sign in to reply
  • Christoph Grüninger added 1 commit

    added 1 commit

    • 7f49950f - Initialize variable theFather

    Compare with previous version

  • Oliver Sander mentioned in commit 46d3657c

    mentioned in commit 46d3657c

  • Oliver Sander merged

    merged

    • Please register or sign in to reply