diff mbox series

[FFmpeg-devel,RFC] tools/patcheck: portability fixes.

Message ID 20230727181552.13387-1-Reimar.Doeffinger@gmx.de
State New
Headers show
Series [FFmpeg-devel,RFC] tools/patcheck: portability fixes. | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Reimar Döffinger July 27, 2023, 6:15 p.m. UTC 
From: Reimar Döffinger <Reimar.Doeffinger@gmx.de>

Enough to make it run on macOS.
In particular:
- fix "empty subexpression" errors caused by constructs like (smth|),
  use ? instead to make them optional
- no -d option for xargs, use the more standard -0 and use tr to
  replace newlines with 0.

Not sure if these cause issues somewhere else, not even completely
sure they all work, but quick testing suggests they work.
On the other hand I remember issues with '?' where I resorted to {0,1}
instead, but I do not remember details.
Ignore if fixing these seems not worth the risk.
---
 tools/patcheck | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Michael Niedermayer July 27, 2023, 8:15 p.m. UTC | #1 
On Thu, Jul 27, 2023 at 08:15:52PM +0200, Reimar.Doeffinger@gmx.de wrote:
> From: Reimar Döffinger <Reimar.Doeffinger@gmx.de>
> 
> Enough to make it run on macOS.
> In particular:
> - fix "empty subexpression" errors caused by constructs like (smth|),
>   use ? instead to make them optional
> - no -d option for xargs, use the more standard -0 and use tr to
>   replace newlines with 0.
> 
> Not sure if these cause issues somewhere else, not even completely
> sure they all work, but quick testing suggests they work.
> On the other hand I remember issues with '?' where I resorted to {0,1}
> instead, but I do not remember details.
> Ignore if fixing these seems not worth the risk.
> ---
>  tools/patcheck | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

making it work on more platforms is a good idea.
It seems still working here on ubuntu but i certainly dont know which
syntax constructs work where

[...]

thx
diff mbox series

Patch

diff --git a/tools/patcheck b/tools/patcheck
index fe52938f29..ee993c60fc 100755
--- a/tools/patcheck
+++ b/tools/patcheck
@@ -21,7 +21,7 @@  echo may or may not be bad. When you use it and it misses something or detects
 echo something wrong, fix it and send a patch to the ffmpeg-devel mailing list.
 echo License: GPL, Author: Michael Niedermayer
 
-ERE_PRITYP='(unsigned *|)(char|short|long|int|long *int|short *int|void|float|double|(u|)int(8|16|32|64)_t)'
+ERE_PRITYP='(unsigned *)?(char|short|long|int|long *int|short *int|void|float|double|u?int(8|16|32|64)_t)'
 ERE_TYPES='(const|static|av_cold|inline| *)*('$ERE_PRITYP'|[a-zA-Z][a-zA-Z0-9_]*)[* ]{1,}[a-zA-Z][a-zA-Z0-9_]*'
 ERE_FUNCS="$ERE_TYPES"' *\('
 
@@ -63,7 +63,7 @@  hiegrep '\+= *1 *;'     'can be simplified to ++' $*
 hiegrep '-= *1 *;'      'can be simplified to --' $*
 hiegrep '((!|=)= *(0|NULL)[^0-9a-z]|[^0-9a-z](0|NULL) *(!|=)=)' 'x==0 / x!=0 can be simplified to !x / x' $*
 
-$EGREP $OPT '^\+ *(const *|)static' $*| $EGREP --color=always '[^=]= *(0|NULL)[^0-9a-zA-Z]'> $TMP && printf '\nuseless 0 init\n'
+$EGREP $OPT '^\+ *(const *)?static' $*| $EGREP --color=always '[^=]= *(0|NULL)[^0-9a-zA-Z]'> $TMP && printf '\nuseless 0 init\n'
 cat $TMP
 hiegrep '# *ifdef * (HAVE|CONFIG)_' 'ifdefs that should be #if' $*
 
@@ -77,7 +77,7 @@  hiegrep ':\+ *'"$ERE_PRITYP"' *inline' 'non static inline or strangely ordered i
 hiegrep "$ERE_FUNCS"' *\)' 'missing void' $*
 hiegrep '(sprintf|strcat|strcpy)' 'Possible security issue, make sure this is safe or use snprintf/av_strl*' $*
 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' $*
-hiegrep '#(el|)if *(0|1)' 'useless #if' $*
+hiegrep '#(el)?if *(0|1)' 'useless #if' $*
 hiegrep 'if *\( *(0|1) *\)' 'useless if()' $*
 hiegrep '& *[a-zA-Z0-9_]* *\[ *0 *\]' 'useless & [0]' $*
 hiegrep '(\( *[0-9] *(&&|\|\|)|(&&|\|\|) *[0-9] *\))' 'overriding condition' $*
@@ -118,22 +118,22 @@  if test -e $TMP ; then
     cat $TMP
 fi
 
-$EGREP -B2 $OPT '^(\+|) *('"$ERE_TYPES"'|# *define)' $* | $EGREP -A2 --color=always '(:|-)\+[^/]*/(\*([^*]|$)|/([^/]|$))' > $TMP && printf "\n Non doxy comments\n"
+$EGREP -B2 $OPT '^\+? *('"$ERE_TYPES"'|# *define)' $* | $EGREP -A2 --color=always '(:|-)\+[^/]*/(\*([^*]|$)|/([^/]|$))' > $TMP && printf "\n Non doxy comments\n"
 cat $TMP
 
 rm $TMP
 for i in \
     $($EGREP -H '^\+ *'"$ERE_TYPES" $*  |\
     $GREP -v '(' | $EGREP -v '\Wgoto\W' |\
-    xargs -d '\n' -n 1 |\
+    tr '\n' '\0' | xargs -0 -n 1 |\
     $GREP -o '[* ][* ]*[a-zA-Z][0-9a-zA-Z_]* *[,;=]' |\
     sed 's/.[* ]*\([a-zA-Z][0-9a-zA-Z_]*\) *[,;=]/\1/') \
     ; do
     echo $i | $GREP '^NULL$' && continue
-    $EGREP $i' *(\+|-|\*|/|\||&|%|)=[^=]' $* >/dev/null || echo "possibly never written:"$i >> $TMP
+    $EGREP $i' *(\+|-|\*|/|\||&|%)?=[^=]' $* >/dev/null || echo "possibly never written:"$i >> $TMP
     $EGREP '(=|\(|return).*'$i'(==|[^=])*$'    $* >/dev/null || echo "possibly never read   :"$i >> $TMP
-    $EGREP -o $i' *((\+|-|\*|/|\||&|%|)=[^=]|\+\+|--) *(0x|)[0-9]*(;|)'   $* |\
-           $EGREP -v $i' *= *(0x|)[0-9]{1,};'>/dev/null || echo "possibly constant     :"$i >> $TMP
+    $EGREP -o $i' *((\+|-|\*|/|\||&|%)?=[^=]|\+\+|--) *(0x)?[0-9]*;?'   $* |\
+           $EGREP -v $i' *= *(0x)?[0-9]{1,};'>/dev/null || echo "possibly constant     :"$i >> $TMP
 done
 if test -e $TMP ; then
     printf '\npossibly unused variables\n'
@@ -151,7 +151,7 @@  cat $TMP | tr '@' '\n'
 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"
 cat $TMP | tr '@' '\n'
 
-cat $* | tr '\n' '@' | $EGREP --color=always -o '\+ *if *\( *([A-Za-z0-9_]*) *\)[ @\\+]*av_free(p|) *\( *(&|) *\1[^-.]'  >$TMP && printf "\nav_free(NULL) is safe\n"
+cat $* | tr '\n' '@' | $EGREP --color=always -o '\+ *if *\( *([A-Za-z0-9_]*) *\)[ @\\+]*av_freep? *\( *&? *\1[^-.]'  >$TMP && printf "\nav_free(NULL) is safe\n"
 cat $TMP | tr '@' '\n'
 
 cat $* | tr '\n' '@' | $EGREP --color=always -o '[^a-zA-Z0-9_]([a-zA-Z0-9_]*) *= *av_malloc *\([^)]*\)[ @;\\+]*memset *\( *\1'  >$TMP && printf "\nav_mallocz()\n"