> -----Original Message-----
> From: Guo, Yejun
> Sent: Wednesday, April 24, 2019 7:02 PM
> To: Guo, Yejun <yejun....@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH V4 2/2] configure: replace 'pr' with printf
> since busybox does not support pr
>
> > log_file(){
> > log BEGIN $1
> > - pr -n -t $1 >> $logfile
> > + i=1
>
> Shell variables are global. This means that any part of the code which ends
> up calling `log_file` directly or through some other function (and not
> isolated
> in a subshell) will get its `i` variable clobbered by `log_file`. This can be
> very tricky to debug and identify.
how about add the keyword local which makes the variables not global.
and I should also add 'local' in function print_in_columns.
>
> Please use a variable name which is unlikely to be used by other functions,
> like `log_file_i`. Same goes for `line` below. This is critical especially in
I'll also add local keyword for line.
local line
while IFS= read -r line;do
...
> functions which end up get called by a lot of other function - like
> `log_file`.
>
> > + while read line;do
>
> This needs to be `while IFS= read -r line; do`.
>
> Without the IFS= part trailing white spaces will be trimmed, and without
> the -r part backslash ('\') will be interpreted an an escape char.
thanks, will change it.
>
> > + printf '%5s %s\n' "${i}" "${line}"
> > + i=$(($i+1))
> > + done < $1 >> $logfile
>
> While the previous version of the function used `$1` unquoted, it's generally
> a very bad practice, as it cannot handle white spaces in file names and is
> subjected to globbing and open for attacks. In general always quote _all_
> strings unless you know for a fact that it's not required. In this case it
> _is_
> required.
will quote $1 and $logfile
>
> > log END $1
> > }
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
