[PATCHES] Some fixes in s4/scripting/upgradeprovision

[Noel Power]

On 10/10/2018 21:36, Douglas Bagnall via samba-technical wrote:
> For no good reason I have been looking at source4/scripting/samba_upgradeprovision,
> fixing a few simple errors like misspelt or unused variables.
>
> I ran aground on this cluster:
>
>          if att == "forceLogoff":
>              ref=0x8000000000000000
>              oldval=int(old[0][att][0])
>              newval=int(new[0][att][0])
>              ref == old and ref == abs(new)
>              return True
>
> 'oldval' and 'newval' are unused.
> 'old' and 'new' are non-numeric, so will never equal 'ref' (1 << 63).
> 'abs(new)' would raise an exception, but is never evaluated because
> 'ref == old' short-circuits though otherwise does nothing.
>
> Perhaps at the end of that series of horrific no-ops, the only thing
> to return is True, but I sort of wonder if this might be intended:
>
> -            ref == old and ref == abs(new)
> -            return True
> +            return ref == oldval and ref == abs(newval)
>
> If we just remove the four lines we lose an exception if the values
> don't parse as int.
>
> Any ideas? (other than wishes for -Wall level static analysis for Python?
>
> cheers,
> Douglas

-            if (long(str(old[0][att])) == 0):
+            if (int(str(old[0][att])) == 0):


very minor nitpick, we should do

-            if (long(str(old[0][att])) == 0):
+            if (int(old[0][att]) == 0):

and remove the attempt to convert to str because it could cause a 
UnicodeDecode error rather than a desired ValueError if the attribute 
value is not a valid string/byte/bytearray representation of a number. I 
don't think anyone is filtering any particular exceptions here so 
probably not a real issue with this piece of code perse

RB+

Noel