LMAO.
-------- Original Message --------
Subject: Re: Exercise of the day: calling my_read() ...
Date: Fri, 31 Jul 2009 18:14:16 +0200
From: Daniel Fischer <[email protected]>
To: [email protected]
CC: Vladislav Vaintroub <[email protected]>, [email protected], 'dev-private'
<[email protected]>, [email protected]
References: <[email protected]>
<008a01ca116d$f7e751c0$e7b5f5...@com> <[email protected]>
<[email protected]>
Sinisa,
5.1 uses (correctly) size_t instead of int's ...
Nah, I disagree. It does use size_t but I would argue it doesn't do so
correctly.
Count is size_t:
size_t my_read(File Filedes, uchar *Buffer, size_t Count, myf MyFlags)
It is passed as an argument to read(2) where a size_t is expected.
if ((readbytes= read(Filedes, Buffer, (uint) Count)) != Count)
Oh wait, we have a size_t and need a size_t but cast it to uint just for
the fun of it? This means that reading Count > UINT_MAX bytes will not
work even if size_t is wider than uint (which it may well be on a 32 bit
system).
On the same line, the result of read(2) is stored in readbytes, which is
size_t. However, the result type of read(2) is ssize_t on most
platforms. Normally size_t is signed too on UNIX so we are in luck. Also
it's likely that ssize_t and size_t would have the same width in any case.
By the way, on the line above the read(2) call I find this:
errno= 0; /* Linux doesn't reset this */
Of course Linux doesn't reset it; no standard-conforming function that
uses errno to return an error will reset it...
Anyway, later, readbytes is cast to int, in order to compare to -1
(which is an int literal in this case):
if ((readbytes == 0 || (int) readbytes == -1) && errno == EINTR)
This means that any read of size S from an interrupted call where S %
UINT_MAX == -1 will be interpreted as a failure and the data is lost, so
we can definitely not trust it with Count > UINT_MAX even if we remove
the blocking cast. Of course this is unlikely to happen in reality
because the syscall has to be interrupted for this to trigger.
In some another place it is cast to long for printing. Since that is
debug code it doesn't really matter:
DBUG_PRINT("debug", ("my_read() was interrupted and returned %ld",
(long) readbytes));
Although it also appears cast to int in debug messages, so we likely
don't know what we want anyway:
DBUG_PRINT("warning",("Read only %d bytes off %lu from %d, errno: %d",
(int) readbytes, (ulong) Count, Filedes,
my_errno));
In the same debug statement, Count is also cast to ulong for printing
even though we cast it to uint for execution and only expect to read a
block of size <= INT_MAX as evidenced by the cast of readbytes to int in
the same message. This doesn't actually break reading > INT_MAX but it
is clear that we don't expect to do read blocks that large. (There is a
typo there in the English text too.)
Elsewhere, it is not cast to int, but instead compared to (size_t) -1:
if (readbytes == (size_t) -1)
This usually works, but if size_t is wider than int, a size_t value of
-1 will not be equal to an int value of -1 cast to size_t, and because
of that the equality test will have a different result.
++
I would not trust this function with anything > INT_MAX, and I'm not
even sure I would trust it with anything > 50 or so.
Daniel
--
Daniel Fischer, MySQL Tech Lead Build +46 18174400 ext. 4537
Sun Microsystems GmbH, Sonnenallee 1, DE-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer: Thomas Schroeder, Wolfang Engels, Wolf Frenkel
Vorsitz d. Aufs.rat.: Martin Haering HRB MUC 161028 49.011, 8.376
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
