Hi Luis,

I agree this block is structured strangely. I had to spend a few minutes 
staring at it to make sense of it. After looking at it, I'm not sure the 
proposed change fixes things properly.

I think line 105 should be 'outdented'; in the original code, the break 
would trigger any time 'att' was in 'be' and in 'self.lattrs'. With your 
proposed change, the break will only occur if att == 'orig_be_name'.

Additionally, I think initializing "att = None" on line 95 is more 
appropriate.

Also, is 'att' used elsewhere in this function? Do we even need to store 
it for later? It looks like it gets grabbed on line 113, but that also 
looks like a typo given the 'for' block at 110-125...

- Keith

Luis de Bethencourt wrote:
> Hi,
>
> Could I please ask for reviewing the changes to fix bug 12532:
> using loop index as variable out of the loop
>
> src/cmd/beadm/BootEnvironment.py
>
>    line 99
>    using att variable which is the index of the for loop. this loop is 
> breaked    to ensure the att variable is in the right element. this is 
> structured
>    weirdly, the 'if att == ' should be inside if in the for loop.
>
>
> Thank you very much,
> Luis
>
> * Webrev
> ======
> http://cr.opensolaris.org/~luisbg/12532/
>
> * Testing
> ======
> * platforms tested: x86
> * run regression tests
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to