Re: [Xen-devel] [PATCH 1/1] tools/hotplug: Scan xenstore once when attaching shared images files

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 01/10/15 16:16, Mike Latimer wrote:
> Hi again,
> 
> On Thursday, October 01, 2015 10:51:08 AM George Dunlap wrote:
>>>
>>> -      if [ "$d" = "$devmm" ]
>>> +      if [[ "$devmm" == *"$d,"* ]]
>>
>> Style nit: using [[ instead of [.  TBH I prefer [[, but it's probably
>> better to be consistent with the rest of the file.
> 
> I was about to change this to something like:
> 
>   if [ "$devmm" != ${devmm/$d/} ]
> 
> but I'm a bit concerned about the potential size of the variables involved. 
> If 
> there are 500 devices with 5-7 characters per device, $devmm becomes a pretty 
> large string. I'm not sure this bash substitution method is a great option. I 
> also don't really want the hit of passing the variable to grep.
> 
> According to one post I found benchmarking various types of tests [1], the 
> two 
> fastest options are:
> 
>   [[ $b == *$a* ]]
> 
>   case $b in *$a):;;esac
> 
> I can implement a case statement, but that seems even less clean than the 
> simple [[ ... ]] approach (since there is only one case we care about).
> 
> As this is a #!/bin/bash script, is [[ ... ]] okay to use, or would you 
> prefer 
> to use the case statement? (If you have any other ideas, I'm open to that as 
> well.)

Oh, right -- sorry, I didn't realize that the test you were using was
only available with [[ ]].

The maintainers (of which I'm not one) have the final say.  I'd
personally be in favor of leaving it [[ ]] with a comment before each
one saying that the $b == *$a* format is only available in [[ ]], so
that no one comes later and "cleans it up".

I'd personally be even more happy if all the [ ] in the script were
magically changed to [[ ]]. :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel