Hi,
I'm currently thinking about various tests we could do on our tree to
avoid regressions, not just in functionality (which is somewhat hard to
do with all the board specific code we have), but also in style and
practices.
My first attempt is the attached script
(lint-001-no-global-config-in-romstage), which checks for preprocessor
symbols defined in mainboard romstages that are used elsewhere in the tree.
As you can see from my other mails these days, I'm currently trying to
cut down on these, by moving them to Kconfig.
This has several reasons: First, it's necessary to have these symbols
available globally when we start building the romstage in more separate
compilation units (more on that below). Second, it's basically a second
configuration mechanism besides the one we have, which is Kconfig.
The script, when run from the top level directory of a coreboot
checkout, emits the symbols that are defined in some romstage and are
used elsewhere. No further information is given as the idea is to give a
relatively quick overview on the tree, and fetching a list of files
where those symbols are in use would be expensive.
I propose to store this script (and similar ones) somewhere under util/,
and hook them up in the Makefile ("make lint"?) and in the autobuilder
(qa.coreboot.org), and have that report failure if they return any output.
That way, no change can readd this "second config system" without being
noticed.
I don't propose to do this now - that would mean that the autobuilder
reports errors for the next couple of weeks, but once we got the tree in
shape that it passes the test of the script, it could go in and make
sure that this issue doesn't come up again.
Another test could ensure that no mainboard code (and maybe even more
parts of the tree later-on) #includes *.c files.
That effort is useful because smaller compilation units make it easier
to track down bugs, to validate code, and so on.
If I ask you, could you tell (without looking) if init_cpus.c or
fidvid.c should be included first on an AMD K8/Fam10 board? It shouldn't
matter, but it does.
It's init_cpus.c, by the way ;-)
External tools, such as lint, splint could also be added into that
framework, should they be actually useful for our code (many aren't
because of their assumptions on C coding within OS environments).
This mail is marked RFC for a reason - what do you think?
Regards,
Patrick
#!/bin/sh
DEFINES=`grep "#define" src/mainboard/*/*/romstage.c |sed 's,.*#define[\t
]\([^\t ]*\)[\t ].*,\1,' | grep -v "(" | sort -u`
SCANBUCKET=`mktemp`
find src -name .svn -type d -prune -o -name mainboard -type d -prune -o -type f
-exec sed -f remccoms3.sed {} + > $SCANBUCKET
for define in $DEFINES; do
if [ `grep -c $define $SCANBUCKET` -gt 0 ]; then
echo "$define is defined in mainboard(s) and used elsewhere"
fi
done
rm -f $SCANBUCKET
#! /bin/sed -nf
# Remove C and C++ comments, by Brian Hiles ([email protected])
# Sped up (and bugfixed to some extent) by Paolo Bonzini ([email protected])
# Works its way through the line, copying to hold space the text up to the
# first special character (/, ", '). The original version went exactly a
# character at a time, hence the greater speed of this one. But the concept
# and especially the trick of building the line in hold space are entirely
# merit of Brian.
# Taken from http://sed.sourceforge.net/grabbag/scripts/remccoms3.sed
# According to http://sed.sourceforge.net/grabbag/ it's in the public domain
# Changes:
# 2010-11-06: Remove strings
:loop
# This line is sufficient to remove C++ comments!
/^\/\// s,.*,,
# addition for coreboot-lint: For our purpose we don't need strings
s,"[^"]*",,g
/^$/{
x
p
n
b loop
}
/^"/{
:double
/^$/{
x
p
n
/^"/b break
b double
}
H
x
s,\n\(.[^\"]*\).*,\1,
x
s,.[^\"]*,,
/^"/b break
/^\\/{
H
x
s,\n\(.\).*,\1,
x
s/.//
}
b double
}
/^'/{
:single
/^$/{
x
p
n
/^'/b break
b single
}
H
x
s,\n\(.[^\']*\).*,\1,
x
s,.[^\']*,,
/^'/b break
/^\\/{
H
x
s,\n\(.\).*,\1,
x
s/.//
}
b single
}
/^\/\*/{
s/.//
:ccom
s,^.[^*]*,,
/^$/ n
/^\*\//{
s/..//
b loop
}
b ccom
}
:break
H
x
s,\n\(.[^"'/]*\).*,\1,
x
s/.[^"'/]*//
b loop
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
