ffmpeg / tools / patcheck @ c7d22c5d
History | View | Annotate | Download (8.21 KB)
1 | 6c2dd77e | Michael Niedermayer | #!/bin/sh |
---|---|---|---|
2 | |||
3 | 09d7da86 | Rafaël Carré | # if no argument provided, write stdin to a file and re-run the script |
4 | if [ $# = 0 ]; then |
||
5 | cat > patcheck.stdout |
||
6 | $0 patcheck.stdout |
||
7 | rm -f patcheck.stdout |
||
8 | exit |
||
9 | fi |
||
10 | |||
11 | ff3be572 | Brad | GREP=grep |
12 | EGREP=egrep |
||
13 | 6c2dd77e | Michael Niedermayer | TMP=patcheck.tmp |
14 | OPT="-nH" |
||
15 | 6252040e | Diego Biurrun | #FILES=$($GREP '^+++' $* | sed 's/+++ //g') |
16 | 6c2dd77e | Michael Niedermayer | |
17 | echo patCHeck 1e10.0 |
||
18 | echo This tool is intended to help a human check/review patches it is very far from |
||
19 | echo being free of false positives and negatives, its output are just hints of what |
||
20 | echo may or may not be bad. When you use it and it misses something or detects |
||
21 | 14622ef0 | Diego Biurrun | echo something wrong, fix it and send a patch to the libav-devel mailing list. |
22 | 6c2dd77e | Michael Niedermayer | echo License:GPL Autor: Michael Niedermayer |
23 | |||
24 | ERE_PRITYP='(unsigned *|)(char|short|long|int|long *int|short *int|void|float|double|(u|)int(8|16|32|64)_t)' |
||
25 | ERE_TYPES='(const|static|av_cold|inline| *)*('$ERE_PRITYP'|[a-zA-Z][a-zA-Z0-9_]*)[* ]{1,}[a-zA-Z][a-zA-Z0-9_]*' |
||
26 | ERE_FUNCS="$ERE_TYPES"' *\(' |
||
27 | |||
28 | hiegrep(){ |
||
29 | arg="$1" |
||
30 | msg="$2" |
||
31 | shift 2 |
||
32 | ff3be572 | Brad | $GREP $OPT '^+' $* | $GREP -v ':+++'| $EGREP --color=always -- "$arg"> $TMP && printf "\n$msg\n" |
33 | 6c2dd77e | Michael Niedermayer | cat $TMP |
34 | } |
||
35 | |||
36 | 3d8f9025 | Michael Niedermayer | hiegrep2(){ |
37 | arg="$1" |
||
38 | varg="$2" |
||
39 | msg="$3" |
||
40 | shift 3 |
||
41 | ff3be572 | Brad | $GREP $OPT '^+' $* | $GREP -v ':+++' | $EGREP -v -- "$varg" | $EGREP --color=always -- "$arg" > $TMP && printf "\n$msg\n" |
42 | 3d8f9025 | Michael Niedermayer | cat $TMP |
43 | } |
||
44 | |||
45 | 6c2dd77e | Michael Niedermayer | hiegrep '[[:space:]]$' 'trailing whitespace' $* |
46 | 6252040e | Diego Biurrun | hiegrep "$(echo x | tr 'x' '\t')" 'tabs' $* |
47 | 6c2dd77e | Michael Niedermayer | #hiegrep ':\+$' 'Empty lines' $* |
48 | hiegrep ';;' 'double ;' $* |
||
49 | 74892f7a | Michael Niedermayer | hiegrep2 '\b_[a-zA-Z0-9_]{1,}' '__(asm|attribute)([^a-zA-Z0-9]|$)' 'reserved identifer' $* |
50 | 6c2dd77e | Michael Niedermayer | hiegrep '//[-/<\* ]*$' 'empty comment' $* |
51 | hiegrep '/\*[-<\* ]*\*/' 'empty comment' $* |
||
52 | hiegrep 'for *\( *'"$ERE_PRITYP"' ' 'not gcc 2.95 compatible' $* |
||
53 | be3e68b2 | Michael Niedermayer | hiegrep '(static|inline|const) *\1' 'duplicate word' $* |
54 | 9e793a24 | Michael Niedermayer | hiegrep 'INIT_VLC_USE_STATIC' 'forbidden ancient vlc type' $* |
55 | 35614edb | Eli Friedman | hiegrep '=[-+\*\&] ' 'looks like compound assignment' $* |
56 | 78db142a | Michael Niedermayer | hiegrep2 '/\*\* *[a-zA-Z0-9].*' '\*/' 'Inconsistently formatted doxygen comment' $* |
57 | 3ea4d942 | Benoit Fouet | hiegrep '; */\*\*[^<]' 'Misformatted doxygen comment' $* |
58 | 6c2dd77e | Michael Niedermayer | |
59 | 3d8f9025 | Michael Niedermayer | hiegrep2 '(int|unsigned|static|void)[a-zA-Z0-9 _]*(init|end)[a-zA-Z0-9 _]*\(.*[^;]$' '(av_cold|:\+[^a-zA-Z_])' 'These functions may need av_cold, please review the whole patch for similar functions needing av_cold' $* |
60 | 6c2dd77e | Michael Niedermayer | |
61 | hiegrep '\+= *1 *;' 'can be simplified to ++' $* |
||
62 | hiegrep '-= *1 *;' 'can be simplified to --' $* |
||
63 | hiegrep '((!|=)= *(0|NULL)[^0-9a-z]|[^0-9a-z](0|NULL) *(!|=)=)' 'x==0 / x!=0 can be simplified to !x / x' $* |
||
64 | |||
65 | ff3be572 | Brad | $EGREP $OPT '^\+ *(const *|)static' $*| $EGREP --color=always '[^=]= *(0|NULL)[^0-9a-zA-Z]'> $TMP && printf '\nuseless 0 init\n' |
66 | 6c2dd77e | Michael Niedermayer | cat $TMP |
67 | hiegrep '# *ifdef * (HAVE|CONFIG)_' 'ifdefs that should be #if' $* |
||
68 | |||
69 | hiegrep '\b(awnser|cant|dont|quantised|quantisation|teh|wont)\b' 'common typos' $* |
||
70 | |||
71 | hiegrep 'av_log\( *NULL' 'Missing context in av_log' $* |
||
72 | hiegrep '[^sn]printf' 'Please use av_log' $* |
||
73 | hiegrep '\bmalloc' 'Please use av_malloc' $* |
||
74 | hiegrep '\) *av_malloc' 'useless casts' $* |
||
75 | hiegrep ':\+ *'"$ERE_PRITYP"' *inline' 'non static inline or strangely ordered inline+static' $* |
||
76 | hiegrep "$ERE_FUNCS"' *\)' 'missing void' $* |
||
77 | hiegrep '(sprintf|strcat|strcpy)' 'Possible security issue, make sure this is safe or use snprintf/av_strl*' $* |
||
78 | hiegrep '/ *(2|4|8|16|32|64|128|256|512|1024|2048|4096|8192|16384|32768|65536)[^0-9]' 'divide by 2^x could use >> maybe' $* |
||
79 | hiegrep '#(el|)if *(0|1)' 'useless #if' $* |
||
80 | hiegrep 'if *\( *(0|1) *\)' 'useless if()' $* |
||
81 | hiegrep '& *[a-zA-Z0-9_]* *\[ *0 *\]' 'useless & [0]' $* |
||
82 | hiegrep '(\( *[0-9] *(&&|\|\|)|(&&|\|\|) *[0-9] *\))' 'overriding condition' $* |
||
83 | 214ee5f5 | Michael Niedermayer | hiegrep '(:\+|,|;)( *|static|\*)*'"$ERE_PRITYP"' *\*( |\*)*(src|source|input|in[^a-z])' 'missing const?' $* |
84 | hiegrep '(:\+|,|;)( *|static|\*)*'"$ERE_PRITYP"' *(src|source|input|in)([0-9A-Z_][0-9A-Za-z_]*){1,} *\[' 'missing const (test2)?' $* |
||
85 | 70ec53a0 | Michael Niedermayer | hiegrep ' *static *'"$ERE_FUNCS"'[^)]*\);' 'static prototype, maybe you should reorder your functions' $* |
86 | c4e12a9f | Michael Niedermayer | hiegrep '@file: *[a-zA-Z0-9_]' 'doxy filetag with filename can in the future cause problems when forgotten during a rename' $* |
87 | c7d22c5d | Peter Ross | hiegrep '\bassert' 'Please use av_assert0, av_assert1 or av_assert2' $* |
88 | 6c2dd77e | Michael Niedermayer | |
89 | 3d8f9025 | Michael Niedermayer | hiegrep2 '\.long_name *=' 'NULL_IF_CONFIG_SMAL' 'missing NULL_IF_CONFIG_SMAL' $* |
90 | 4a0cf686 | Daniel Verkamp | hiegrep2 '\.pix_fmts *= *\(' 'const' 'missing const for pix_fmts array' $* |
91 | hiegrep2 '\.sample_fmts *= *\(' 'const' 'missing const for sample_fmts array' $* |
||
92 | hiegrep2 '\.supported_framerates *= *\(' 'const' 'missing const for supported_framerates array' $* |
||
93 | hiegrep2 '\.channel_layouts *= *\(' 'const' 'missing const for channel_layouts array' $* |
||
94 | 6c2dd77e | Michael Niedermayer | |
95 | ff3be572 | Brad | #$EGREP $OPT '^\+.*const ' $*| $GREP -v 'static'> $TMP && printf '\nnon static const\n' |
96 | 6c2dd77e | Michael Niedermayer | #cat $TMP |
97 | |||
98 | 3d8f9025 | Michael Niedermayer | hiegrep2 "$ERE_TYPES" '(static|av_|ff_|typedef|:\+[^a-zA-Z_])' 'Non static with no ff_/av_ prefix' $* |
99 | 6c2dd77e | Michael Niedermayer | |
100 | 0f485a9c | Michael Niedermayer | hiegrep ':\+[^}#]*else' 'missing } prior to else' $* |
101 | 54ce8a4c | Michael Niedermayer | hiegrep '(if|while|for)\(' 'missing whitespace between keyword and ( (feel free to ignore)' $* |
102 | hiegrep '(else|do){' 'missing whitespace between keyword and { (feel free to ignore)' $* |
||
103 | hiegrep '}(else|while)' 'missing whitespace between } and keyword (feel free to ignore)' $* |
||
104 | 6c2dd77e | Michael Niedermayer | |
105 | #FIXME this should print the previous statement maybe |
||
106 | hiegrep ':\+ *{ *$' '{ should be on the same line as the related previous statement' $* |
||
107 | |||
108 | |||
109 | rm $TMP |
||
110 | 6252040e | Diego Biurrun | for i in $($GREP -H '^+.*@param' $*| sed 's/^\([^:]*\):.*@param\(\[.*\]\|\) *\([a-zA-Z0-9_]*\) .*$/\1:\3/') ; do |
111 | doxpar=$(echo $i | sed 's/^.*:\(.*\)$/\1/') |
||
112 | file=$(echo $i | sed 's/^\([^:]*\):.*$/\1/') |
||
113 | ff3be572 | Brad | $GREP " *$doxpar *[),]" $file | $GREP -v '@param' >/dev/null || $GREP --color=always "@param *$doxpar" $file >>$TMP |
114 | 6c2dd77e | Michael Niedermayer | done |
115 | if test -e $TMP ; then |
||
116 | f11c9ee9 | Alex Converse | printf '\nmismatching doxy params\n' |
117 | 6c2dd77e | Michael Niedermayer | cat $TMP |
118 | fi |
||
119 | |||
120 | ff3be572 | Brad | $EGREP -B2 $OPT '^(\+|) *('"$ERE_TYPES"'|# *define)' $* | $EGREP -A2 --color=always '(:|-)\+[^/]*/(\*([^*]|$)|/([^/]|$))' > $TMP && printf "\n Non doxy comments\n" |
121 | 6c2dd77e | Michael Niedermayer | cat $TMP |
122 | |||
123 | rm $TMP |
||
124 | for i in \ |
||
125 | 6252040e | Diego Biurrun | $($EGREP -H '^\+ *'"$ERE_TYPES" $* |\ |
126 | ff3be572 | Brad | $GREP -v '(' | $EGREP -v '\Wgoto\W' |\ |
127 | 6c2dd77e | Michael Niedermayer | xargs -d '\n' -n 1 |\ |
128 | ff3be572 | Brad | $GREP -o '[* ][* ]*[a-zA-Z][0-9a-zA-Z_]* *[,;=]' |\ |
129 | 6252040e | Diego Biurrun | sed 's/.[* ]*\([a-zA-Z][0-9a-zA-Z_]*\) *[,;=]/\1/') \ |
130 | 6c2dd77e | Michael Niedermayer | ; do |
131 | ff3be572 | Brad | echo $i | $GREP '^NULL$' && continue |
132 | $EGREP $i' *(\+|-|\*|/|\||&|%|)=[^=]' $* >/dev/null || echo "possibly never written:"$i >> $TMP |
||
133 | $EGREP '(=|\(|return).*'$i'(==|[^=])*$' $* >/dev/null || echo "possibly never read :"$i >> $TMP |
||
134 | $EGREP -o $i' *((\+|-|\*|/|\||&|%|)=[^=]|\+\+|--) *(0x|)[0-9]*(;|)' $* |\ |
||
135 | $EGREP -v $i' *= *(0x|)[0-9]{1,};'>/dev/null || echo "possibly constant :"$i >> $TMP |
||
136 | 6c2dd77e | Michael Niedermayer | done |
137 | if test -e $TMP ; then |
||
138 | f11c9ee9 | Alex Converse | printf '\npossibly unused variables\n' |
139 | 6c2dd77e | Michael Niedermayer | cat $TMP |
140 | fi |
||
141 | |||
142 | ff3be572 | Brad | $GREP '^+++ .*Changelog' $* >/dev/null || printf "\nMissing changelog entry (ignore if minor change)\n" |
143 | 6c2dd77e | Michael Niedermayer | |
144 | ff3be572 | Brad | cat $* | tr '\n' '@' | $EGREP --color=always -o '(fprintf|av_log|printf)\([^)]*\)[+ ;@]*\1' >$TMP && printf "\nMergeable calls\n" |
145 | 6c2dd77e | Michael Niedermayer | cat $TMP | tr '@' '\n' |
146 | |||
147 | ff3be572 | Brad | cat $* | tr '\n' '@' | $EGREP --color=always -o '\+ *if *\( *([A-Za-z0-9_]*) *[<>]=? *[0-9]* *\) * \1 *= *[0-9]* *;[ @\\+]*else *if *\( *\1 *[<>]=? *[0-9]* *\) *\1 *= *[0-9]* *;' >$TMP && printf "\nav_clip / av_clip_uint8 / av_clip_int16 / ...\n" |
148 | 6c2dd77e | Michael Niedermayer | cat $TMP | tr '@' '\n' |
149 | |||
150 | ff3be572 | Brad | cat $* | tr '\n' '@' | $EGREP --color=always -o '\+ *if *\( *([A-Za-z0-9_]*) *[<>]=? *([A-Za-z0-9_]*) *\)[ @\\+]*(\1|\2) *= *(\1|\2) *;' >$TMP && printf "\nFFMIN/FFMAX\n" |
151 | 6c2dd77e | Michael Niedermayer | cat $TMP | tr '@' '\n' |
152 | |||
153 | ff3be572 | Brad | cat $* | tr '\n' '@' | $EGREP --color=always -o '\+ *if *\( *([A-Za-z0-9_]*) *\)[ @\\+]*av_free(p|) *\( *(&|) *\1[^-.]' >$TMP && printf "\nav_free(NULL) is safe\n" |
154 | 6c2dd77e | Michael Niedermayer | cat $TMP | tr '@' '\n' |
155 | |||
156 | ff3be572 | Brad | cat $* | tr '\n' '@' | $EGREP --color=always -o '[^a-zA-Z0-9_]([a-zA-Z0-9_]*) *= *av_malloc *\([^)]*\)[ @;\\+]*memset *\( *\1' >$TMP && printf "\nav_mallocz()\n" |
157 | 6c2dd77e | Michael Niedermayer | cat $TMP | tr '@' '\n' |
158 | |||
159 | |||
160 | # doesnt work |
||
161 | ff3be572 | Brad | #cat $* | tr '\n' '@' | $EGREP -o '[^a-zA-Z_0-9]([a-zA-Z][a-zA-Z_0-9]*) *=[^=].*\1' | $EGREP -o '[^a-zA-Z_0-9]([a-zA-Z][a-zA-Z_0-9]*) *=[^=].*\1 *=[^=]' >$TMP && printf "\nPossibly written 2x before read\n" |
162 | 6c2dd77e | Michael Niedermayer | #cat $TMP | tr '@' '\n' |
163 | |||
164 | exit |
||
165 | |||
166 | TODO/idea list: |
||
167 | |||
168 | for all demuxers & muxers |
||
169 | ff3be572 | Brad | $EGREP for "avctx->priv_data" |
170 | 6c2dd77e | Michael Niedermayer | |
171 | vertical align = |
||
172 | /* and * align |
||
173 | arrays fitting in smaller types |
||
174 | variables written to twice with no interspaced read |
||
175 | memset(block, 0, 6*64*sizeof(DCTELEM)); -> clear_blocks |
||
176 | check existence of long_name in AVCodec |
||
177 | check that the patch does not touch codec & (de)muxer layer at the same time ->split |
||
178 | |||
179 | write a regression test containing at least a line that triggers each warning once |