| Message ID | 9205d7a3e6fa64aa86ddc99d88b84008213015e7.1486562055.git-series.kieran.bingham@ideasonboard.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Kieran, Thank you for the patch. On Wednesday 08 Feb 2017 14:03:57 Kieran Bingham wrote: > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Parameters can contain characters not suited to use in filenames. > > Add '=','(', and ')' to the filtering, and replace with '_' What's the issue with those characters ? :-) > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > scripts/vsp-lib.sh | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh > index 5aff30217a27..08bf8f36c582 100755 > --- a/scripts/vsp-lib.sh > +++ b/scripts/vsp-lib.sh > @@ -263,6 +263,9 @@ compare_frames() { > local params=${args// /-} > params=${params:+-$params} > params=${params//\//_} > + params=${params/=/_} > + params=${params/(/_} > + params=${params/)/_} According to the bash manpage, "If pattern begins with /, all matches of pattern are replaced with string. Normally only the first match is replaced." Shouldn't you add a leading / as for the \/ substitution ? > params=$in_fmt-$out_fmt-$size$params > > if [ x$__vsp_pixel_perfect != xtrue ] ; then
Hi Laurent, Thanks for the review! On 10/02/17 07:58, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Wednesday 08 Feb 2017 14:03:57 Kieran Bingham wrote: >> From: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> Parameters can contain characters not suited to use in filenames. >> >> Add '=','(', and ')' to the filtering, and replace with '_' > > What's the issue with those characters ? :-) Actually, now I think about it; the real issue is they break my automated file conversion scripts. The '=' breaks Makefile wildcard matching, and the '(', just cause extra escaping to be required on the command line, when I try to examine the output files. It could easily be argued that those cases could be handled elsewhere, but it seemed easy to 'clean' the filename. >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> scripts/vsp-lib.sh | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh >> index 5aff30217a27..08bf8f36c582 100755 >> --- a/scripts/vsp-lib.sh >> +++ b/scripts/vsp-lib.sh >> @@ -263,6 +263,9 @@ compare_frames() { >> local params=${args// /-} >> params=${params:+-$params} >> params=${params//\//_} >> + params=${params/=/_} >> + params=${params/(/_} >> + params=${params/)/_} > > According to the bash manpage, > > "If pattern begins with /, all matches of pattern are replaced with string. > Normally only the first match is replaced." > > Shouldn't you add a leading / as for the \/ substitution ? I tried that when I copied the existing line, but it caused breakage. But of course my shell is 'busybox sh' and not bash ... Hrm ... I've just retested on a local script, and it worked fine. # ./k.sh abc=def(ghi)jkl abc_def(ghi)jkl abc_def_ghi)jkl abc_def_ghi_jkl ... So I must have been trying to work around a 'non-error' :D -- Kieran >> params=$in_fmt-$out_fmt-$size$params >> >> if [ x$__vsp_pixel_perfect != xtrue ] ; then >
diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh index 5aff30217a27..08bf8f36c582 100755 --- a/scripts/vsp-lib.sh +++ b/scripts/vsp-lib.sh @@ -263,6 +263,9 @@ compare_frames() { local params=${args// /-} params=${params:+-$params} params=${params//\//_} + params=${params/=/_} + params=${params/(/_} + params=${params/)/_} params=$in_fmt-$out_fmt-$size$params if [ x$__vsp_pixel_perfect != xtrue ] ; then