Kristian Nielsen wrote:
Philip Herron <[email protected]> writes:
@@ -149,11 +149,11 @@
while (1)
{
off_t length;
- char *buffer= NULL;
+ char *buffer=(char*)malloc(sizeof(char));
char *temp_buffer;
/* Read the size */
- if (read(file, &length, sizeof(uint64_t)) != sizeof(uint64_t))
+ if (read(file, &length, sizeof(uint32_t)) != sizeof(uint32_t))
break;
temp_buffer= (char *)realloc(buffer, length);
I guess its because the length is off_t which is just: typedef long off_t
so its still 32bit and therefore too small to read a 64bit int into or
something along those lines.
Well, long is 32 bit on 32-bit Linux, but 64 bit on 64-bit linux. So this is
going to fail on 64-bit (big-endian for sure, and probably also
little-endian).
And surely the actual size of the number to read depends not on whatever
sizeof(off_t) is, but on what is actually in the file being read? So why not
read the value into a tmp variable of correct size (uint32_t or uint64_t
depending on file format), and afterwards assign to length (checking for
overflow if needed)?
Actually... off_t is the appropriate type for file reading/writing. It
is not necessarily just typedef'd to long. It depends on some
pre-processor macros and some other things... but for proper large-file
support, it is correct to use off_t and let the standard library do the
things it needs to do to accomplish this. (On 32-bit linux it should
_not_ be typedef'd to long, for instance- if it is, please let me know
so we can re-define things we need to as part of configure)
Depending on what we're doing, it may be still be appropriate to read
into a temp var as Kristian suggests and then test for overflow. We have
to do this in places where we are mallocing a size_t variable as well
(size_t is the correct type size for malloc) Especially where we are
doing buffering, because you can open a 64-bit file on 32-bit linux, but
you cannot then malloc 64-bits worth of memory. There are many places in
the code where we assume that we can do that (and blindly cast away the
warning), and it really should be checked for overflow.
Also, from above, realloc() should accept NULL as an argument. If it
gets NULL, it behaves as if malloc were called - so mallocing a
sizeof(char) initially is not required. If you found that it is, please
let me know what platform you are on, as it is a bug and one that we
should test for in configure and possible take action to deal with
systemically.
Monty
Hope this helps,
- Kristian.
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
