List:Commits« Previous MessageNext Message »
From:Jørgen Løland Date:February 13 2009 7:45am
Subject:Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2764)
Bug#39109
View as plain text  
Rafal Somla wrote:
> Hi
> 
> Jørgen Løland wrote:
>>>> @@ -737,11 +806,25 @@ uint Image_info::ts_count() const
>>>>  
>>>>  /// Returns total number of tables in the image.
>>>>  inline
>>>> -ulong Image_info::table_count() const
>>>> +uint Image_info::table_count() const
>>>
>>> [6] Why did you change to uint from ulong? Will this cause issues or 
>>> are you fixing issues?
>>
>> This is a fix. The method returns the value of a private variable of 
>> type ulong. I think the method signature should use the same number of 
>> bits for the return value.
>>
> 
> I'd rather fix it by changing the type of m_table_count to ulong. The 
> original reason for using ulong here was that on 16-bit platforms uint 
> limits the number of tables per snapshot to 64k which, in my opinion, 
> did not leave a big enough margin. True that it is hard to find 16-bit 
> platforms these days, but why not to write "safer" code if it costs 
> nothing but a conscious choice of a type.
> 
> As for return types for other *_count() functions, they are different 
> for different kinds of objects. For example snap_count() returns ushort 
> because there are always at most 256 snapshots per image. Db_count() 
> returns uint because there are unlikely to be more than 64k databases 
> per snapshot (disputable assumption, I can agree).
> 
> If we want to make return types uniform, then I'd rather change all of 
> them to ulong.

I can change all the variables to ulong if you so prefer, but note that 
  this change wasn't about reducing the number of bits used for table 
counts. It was about returning the same type as the variable already had.

I'll change all counts to ulong in a final commit after you have 
reviewed the rest of the patch.

-- 
Jørgen Løland
Thread
bzr commit into mysql-6.0-backup branch (jorgen.loland:2764) Bug#39109Jorgen Loland11 Feb
  • Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2764)Bug#39109Chuck Bell11 Feb
    • Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2764)Bug#39109Jørgen Løland12 Feb
      • Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2764)Bug#39109Rafal Somla13 Feb
        • Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2764)Bug#39109Jørgen Løland13 Feb