[f21/master v2] Avoid the possibility of size variables being unset (#1146585)

David Shea dshea at redhat.com
Thu Sep 25 19:19:46 UTC 2014


On 09/25/2014 02:35 PM, Anne Mulhern wrote:
>
>
>
> ----- Original Message -----
>> From: "David Shea" <dshea at redhat.com>
>> To: anaconda-patches at lists.fedorahosted.org
>> Sent: Thursday, September 25, 2014 1:34:05 PM
>> Subject: [f21/master v2] Avoid the possibility of size variables being unset	(#1146585)
>>
>> Raise an error if part and logvol sizes and maxsizes cannot be
>> converted to integers. Treat a missing size in logvol as an error, and a
>> missing size in part as a request for a default size to be determined by
>> blivet.
>> ---
>>   pyanaconda/kickstart.py | 28 ++++++++++++----------------
>>   1 file changed, 12 insertions(+), 16 deletions(-)
>>
>> diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
>> index fadc501..fe7011a 100644
>> --- a/pyanaconda/kickstart.py
>> +++ b/pyanaconda/kickstart.py
>> @@ -772,11 +772,9 @@ class LogVolData(commands.logvol.F20_LogVolData):
>>           else:
>>               try:
>>                   size = Size("%d MiB" % self.size)
>> -            except ValueError:
>> +            except (ValueError, TypeError):
>>                   raise KickstartValueError(formatErrorMsg(self.lineno,
>>                           msg="The size \"%s\" is invalid." % self.size))
>> -            except TypeError:
>> -                pass
>>   
>>           if self.mountpoint == "swap":
>>               ty = "swap"
>> @@ -923,11 +921,9 @@ class LogVolData(commands.logvol.F20_LogVolData):
>>               if self.maxSizeMB:
>>                   try:
>>                       maxsize = Size("%d MiB" % self.maxSizeMB)
>> -                except ValueError:
>> +                except (ValueError, TypeError):
>>                       raise KickstartValueError(formatErrorMsg(self.lineno,
>>                               msg="The maximum size \"%s\" is invalid." %
>>                               self.maxSizeMB))
>> -                except TypeError:
>> -                    pass
>>               else:
>>                   maxsize = None
>>   
>> @@ -1014,13 +1010,15 @@ class PartitionData(commands.partition.F18_PartData):
>>   
>>           storage.doAutoPart = False
>>   
>> -        try:
>> -            size = Size("%d MiB" % self.size)
>> -        except ValueError:
>> -            raise KickstartValueError(formatErrorMsg(self.lineno,
>> -                    msg=_("The size \"%s\" is invalid.") % self.size))
>> -        except TypeError:
>> -            pass
>> +        if self.size:
>> +            try:
>> +                size = Size("%d MiB" % self.size)
>> +            except (ValueError, TypeError):
>> +                raise KickstartValueError(formatErrorMsg(self.lineno,
>> +                        msg=_("The size \"%s\" is invalid.") % self.size))
>> +        else:
>> +            # Have blivet determine a default value
>> +            size = None
>>   
>>           if self.onbiosdisk != "":
>>               for (disk, biosdisk) in storage.eddDict.iteritems():
>> @@ -1186,11 +1184,9 @@ class PartitionData(commands.partition.F18_PartData):
>>           if self.maxSizeMB:
>>               try:
>>                   maxsize = Size("%d MiB" % self.maxSizeMB)
>> -            except ValueError:
>> +            except (ValueError, TypeError):
>>                   raise KickstartValueError(formatErrorMsg(self.lineno,
>>                           msg=_("The maximum size \"%s\" is invalid.") %
>>                           self.maxSizeMB))
>> -            except TypeError:
>> -                pass
>>           else:
>>               maxsize = None
>>   
>> --
>> 2.1.0
>>
>> _______________________________________________
>> anaconda-patches mailing list
>> anaconda-patches at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>>
> The parser in pykickstart defines these values as ints.
> I think that means that an exception will already have
> been thrown by parser if they can not be converted to ints.
> So I don't understand what is the expected situation here.

Well if that's the case then I guess we can get rid of all of the 
TypeError checks, since they're just there (I think) to catch cases when 
the % operation fails to convert self.size to an int.
>
> The stack trace just looks to me like somebody said "size" when they meant self.size.

I think size is correct, since it's supposed to use the version of size 
converted to a Size at the top of the method, but it was possible at the 
top of the method to get a TypeError if self.size was None or an empty 
string or whatever and then not set the size variable.




More information about the anaconda-patches mailing list