[petsc-dev] Source cleanup potpourri

[Karl Rupp]

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