[OpenAFS-devel] warnings fix

Marcus Watts mdw@umich.edu
Tue, 14 Jul 2009 17:56:32 -0400


I promised last week to post a warnings patch.  Apparently I was too
successful at shaming the gatekeepers, because they've since committed a
lot of warnings fixes to git.  That means my patch is already obselete.
Since there's one of me and many gatekeepers, there's no point my
trying to chase a moving target.  Presumably the most valuable thing
about this patch today is as a bench mark - "here's what can be easily
fixed".

1. patches and files
2. sample build output
3. warning summary, by warning
4. warning summary, by directory
5. some notable surprises
6. too complicated to fix

____ 1. patches and files

Patch files:
/afs/umich.edu/group/itd/build/mdw/openafs/patches/openafs-h20090621-linux1.diff
	small fix to apply to cvs head 2009/06/21 to actually build
/afs/umich.edu/group/itd/build/mdw/openafs/patches/openafs-h20090621-wfix5.patch.bz2
	the "warnings" patch itself
/afs/umich.edu/group/itd/build/mdw/openafs/patches/afs-rxk5-wfix5-hk34.patch.bz2
	patch to go from 'warnings' to the latest build of rxk5.
/afs/umich.edu/user/m/d/mdw/wp/openafs.19
	detailed list of changes in the patch.

____ 2. sample build output

Actual build output,
/afs/umich.edu/group/itd/build/mdw/openafs/tmp/errors.1560
	almost stock 1.5.60 build
		10705 lines of build output
		4830 warnings
		3053 unique source lines causing warnings
/afs/umich.edu/group/itd/build/mdw/openafs/tmp/errors.wf1
	"cvs head 2009/06/21 + linux1.diff"
		10508 lines of build output
		4759 warnings
		3018 unique source lines causing warnings
/afs/umich.edu/group/itd/build/mdw/openafs/tmp/errors.wf5
	with wfix5 patch
		4267 lines of build output
		396 warnings
		208 unique source lines causing warnings
/afs/umich.edu/group/itd/build/mdw/openafs/tmp/errors.hk34
	rxk5 hk34 = "cvs head 2009/06/21 + rxk5"
		4660 lines of build output
		405 warnings
		209 unique source lines causing warnings

Needless to say, it's a lot easier to find new problems from
200 warnings than from 3000 warnings.  So hk34 includes additional
improvements to rxk5/k5ssl that were simply not obvious before.

____ 3. warning summary, by warning

This doesn't reduce warnings to 0.  Some warning are architecture
and compiler specific, and changes to eliminate some problems are
way more complicated.  The intent is to reduce warnings to a small
enough number that new problems are practical to sort out.

Here is a summary of some of the more interesting warnings,
				1560	wf1	wf5	hk34
old-style function definition	339	476	9	9
function dcl isn't a proto	725	1054	24	24
implicit dcl of a funct		577	749	19	19
suggest () around truth		111	121	0	0
return with no value in non	8	46	0	0
format .*expects type		116	613	22	22

____ 4. warning summary, by directory

Here is a summary of warnings by directory
		1560	wf1	wf5	hk34
afs			36	2	2
afs/UKERNEL		140	
afs/VNOPS		8	26	26
afsd		5	5	1	1
afsmonitor		260
aklog			8	1	1
auth		5	13
bozo		33	33
bubasics	3	3	2	2
bucoord		48	48	10	10
budb		12	12	4	4
butc		662	662	8	8
butm		14	14
cmd		1	1	
comerr		3	11	
des		11	16	
dir		14	33	
fsint		52	69	69	69
kauth		293	356	38	29
libacl		2	3
libadmin/adminutil 4	4	1	1
libadmin/bos	33	33	33	33
libadmin/cfg	9	9
libadmin/client	1	1
libadmin/kas	3	3	1	1
libadmin/pts	10	10	3	3
libadmin/samples 36	36	2	2
libadmin/test	56	56	2	2
libadmin/vos	18	18	4	4
...MODLOAD	13	68	26	27
lwp		1	7
pam			14
ptserver	53	67	16	16
rx		68	121	8	13
rxgen		3	10
rxkad		57	84	3	3
rxstat		2	3	3	3
sys		10	16	3	3
ubik		30	37	16	16
uss		26	26	10	10
util		20	35	3	3
venus			611	23	23
venus/test		70
viced		311	311	3	3
vlserver	466	466	12	12
vol		59	86	3	3
volser		352	504	42	42
xstat			147	1	1

____ 5. some notable surprises

openafs.19 also lists 2 more categories that may be of interest,
"Interesting surprises" and "too complicated to fix here".

Many of the interesting surprises were in the backup logic.  Apparently
people who use "cvs head" don't use the afs backup utilities, or
at least, don't stress it much.

Also of interest were error and debug messages which were broken,
including at least one which appeared capable of causing "vos copy/shadow"
to core dump in certain circumstances.

____ 6. too complicated to fix

The most interesting "too complicated to fix here" problem had to do
with fids.  These get passed around as pointers to arrays, pointers to
strutures, pointers to the start of various other structures -- it's
really quite amazing.  There was talk about 64-bit volumeids.  Whether
that's a good idea or not; it ain't happening until this gets fixed.

I made an attempt to use more "const".  This is a bit ugly, but
the potential win is the ability to trap modifications to data that
should not change.  However, a big problem is rxgen; it
doesn't reocrd in its prototypes which parameters will be changed.
For "in" params, it would be nice if it made them "const".
This is tricky because there could be pointers involved, so where
"const" goes gets a bit tricky.  Also, there's a difference between
the client & server sides, which the current rxgen code doesn't clearly
distinguish.  Another consequence is that all of the server side functions
will have slightly different prototypes.

The generic ubik calls show up as warnings, but that's a known problem.
The only clean solution is to get rid of them.  Speaking of which,
the ubik stuff that rxgen does is, um, not very elegant?

					-Marcus Watts