[petsc-dev] Source cleanup potpourri

[Jed Brown]

On Mon, Feb 11, 2013 at 4:06 PM, Karl Rupp <rupp at mcs.anl.gov> wrote:

> * We have a couple of malloc/free in use:
> http://krupp.iue.tuwien.ac.at/**petsc-style/malloc-free.txt<http://krupp.iue.tuwien.ac.at/petsc-style/malloc-free.txt>
> Except for the actual implementation of PetscMalloc() and the like
> somewhere in src/sys/, is there anything I have to keep in mind when fixing
> the other uses?
>

The cases in mesh.c and plex.c are freeing memory allocated by an external
library.

options.c can be processed before the correct malloc implementation has
been determined (because it can be a command-line option).

str.c stuff can be called before PetscInitialize

hash.h is a third-party library that was brought in and was not fully
petsc-fied.

yamlimpls.c is freeing memory allocated by an external library

The tfs stuff was contributed and never petsc-fied. I don't think it's
worth doing maintenance on because it will have to be substantially
rewritten if someone wants to extend it in a nontrivial way.

That pretty much covers it, so I think PETSc is okay on this front.


>
> * 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.


>
> * 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. 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.


>
> * 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.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130211/630710ca/attachment.html>