[Maria-developers] Handling size_t -> int conversions
I'm working on getting rid of the compiler warnings on 64 bit Windows. There are literally thousands of places, where we store a size_t in an int, and the compiler warns about possible loss of data. I'm trying to figure out how to handle this in the best way, and I need a bit of advice on how to proceed. First, is it acceptable to change all/most of these vars from int to size_t? In places where there is a local variable that stores the difference between two pointers, it's fine. But what about places like struct z_stream_s in zlib.h? How do you guys prefer to see patches on this - one big patch, or several smaller patches? Bo Thorsen. Monty Program AB. -- MariaDB: MySQL replacement Community developed. Feature enhanced. Backward compatible.
Hi, Bo! On Aug 18, Bo Thorsen wrote:
I'm working on getting rid of the compiler warnings on 64 bit Windows.
There are literally thousands of places, where we store a size_t in an int, and the compiler warns about possible loss of data.
I'm trying to figure out how to handle this in the best way, and I need a bit of advice on how to proceed.
First, is it acceptable to change all/most of these vars from int to size_t?
I cannot say. I can only answer on a case by case basis.
In places where there is a local variable that stores the difference between two pointers, it's fine.
A difference between two pointers is, strictly speaking, ptrdiff_t.
But what about places like struct z_stream_s in zlib.h?
What member in it is causing problems ? Anyway, I wouldn't try to maintain a better zlib than the official zlib is. That is, for zlib warnings I'd 1. checked if the new zlib version still has the problem 2. if no - upgraded 3. if yes - added a suppression for a warning. Or a cast (as suppression will probably not work on windows)
How do you guys prefer to see patches on this - one big patch, or several smaller patches?
One big patch is fine. But if you will upgrade zlib or readline - this should be in a separate patch. Regards, Sergei
On Wed, 18 Aug 2010 15:25:22 +0200, Bo Thorsen <bo@askmonty.org> wrote:
How do you guys prefer to see patches on this - one big patch, or several smaller patches?
From our experience doing this in Drizzle, smaller patches are better.
You will, inevitably, screw up somewhere. It's just the way things go. Better to be able to bisect back to the problem then go "somewhere in this 600kb diff". -- Stewart Smith
Hi!
"Bo" == Bo Thorsen <bo@askmonty.org> writes:
Bo> I'm working on getting rid of the compiler warnings on 64 bit Windows. Bo> There are literally thousands of places, where we store a size_t in an Bo> int, and the compiler warns about possible loss of data. Bo> I'm trying to figure out how to handle this in the best way, and I need Bo> a bit of advice on how to proceed. Bo> First, is it acceptable to change all/most of these vars from int to Bo> size_t? In places where there is a local variable that stores the Bo> difference between two pointers, it's fine. But what about places like Bo> struct z_stream_s in zlib.h? We should over time fix all these warnings, but we have to do so that we can still relatively easly do merges. In other words, we should do the least amount of changes that fixes most of the code. This means that we should probably first concentrate on fixing structs then mysys and strings library and only then other cases. All cases of between pointers to different objects should be fixed (these are potential bugs) As one can't have have strings > 4G in MySQL it's not critical that we fix all size_t issues as these can't cause any bugs. However getting rid of warnings is always a good thing as having many warnings will cause people to miss the important warnings. We should probably not fix this in zlib.h as we may want to be binary compatible with the normal zlib library. Bo> How do you guys prefer to see patches on this - one big patch, or Bo> several smaller patches? One patch per module would be preferably. Regards, Monty
participants (4)
-
Bo Thorsen -
Michael Widenius -
Sergei Golubchik -
Stewart Smith