Hi Jed, > * We have a couple of malloc/free in use: > > (...) > > That pretty much covers it, so I think PETSc is okay on this front.
Thanks, good news then. PETSc-fication should not be forgotten, though... > * CHKERRQ is missing at a couple of places: > http://krupp.iue.tuwien.ac.at/__petsc-style/ierr-chkerr.txt > <http://krupp.iue.tuwien.ac.at/petsc-style/ierr-chkerr.txt> > I noticed that PetscPrintf() is hardly ever checked for the value of > ierr. Is this intentional? If yes, which other functions in PETSc > should not be checked for the return value in ierr = ...? > > > It looks like most of these are accidentally missing. CHKERRABORT can be > used when the function does not return PetscErrorCode, but the function > should really be fixed to return PetscErrorCode. The current check only covers cases where the error code is stored in ierr. More of PETSc-specific static code analysis could be done with compiler tools, but we are not there yet. > * There is quite some dead code around: > http://krupp.iue.tuwien.ac.at/__petsc-style/if0.txt > <http://krupp.iue.tuwien.ac.at/petsc-style/if0.txt> > Are there any objections (or even LOUD objections) in removing all > blocks which are older than ~1 year? > > > This may need to be done on a case by case basis, but most of it can > probably go. Ok, I will apply defensive reasoning then... > I wouldn't put any effort into > src/dm/impls/{mesh,cartesian} because I suspect that code will be > deleted en masse within a year. I just added a reminder to my calendar for rechecking in two years ;-) > * src/contrib/markadamscolor/__color.c is the only place using assert(): > http://krupp.iue.tuwien.ac.at/__petsc-style/assert.txt > <http://krupp.iue.tuwien.ac.at/petsc-style/assert.txt> > Since color.c entered in the previous century and hasn't been > notably touched since at least 2001, is there any reason other than > software archeology for keeping it? > > > I don't know what other people think about the contrib directory. It has > a lot of cobwebs in it, but it can be fun to look at sometimes. The > FUN3D code in there is now out of date with respect to PETSc and METIS > interfaces. Somewhere along the line, comp/flow.c became syntactically > incorrect. (Perhaps reformatting interacted poorly its heavy use of macros.) That code was hard to refactor, and it seems to be missing a brace somewhere. Anyway, there's still Mercurial as a last resort... ;-) Best regards, Karli
