On 12/07/07 02:46 +0200, Peter Stuge wrote:
On Wed, Jul 11, 2007 at 05:38:41PM -0600, Jordan Crouse wrote:
- int size;
u32 size maybe?
I guess, though I'll bet this code doesn't survive long enough to see those 2G ROM chips.. :)
I won't take that bet.
But it is nice to show that we have thought about it by being explicit. Eliminating potential portability problems is another plus.
Agreed - I'll make the change everywhere.
- p = (unsigned int *) (ptr + BOOTBLOCK_SIZE - 12);
- p[0] = size;
+}
Then there's the matter of the pointer arithmetic again. Since ptr is uchar * this will work, but then writing size to p[0] will write a different number of bytes on different archs, right?
sizeof(unsigned int) is 4 on all the platforms we care about.
640k will be enough. ;)
Heh. Fair enough - depending on compilers to obey convention is probably not a smart idea. I'll switch over to u8 and u32 universally - then we'll be absolutely positive that we're !x86 friendly.
- err:
- if (lar->fd >= 0)
close(lar->fd);- unlink(archive);
- if (lar)
free(lar);If lar can be 0 at err: then the fd deref is a segfault waiting to happen.
Lar can't be 0 at err
Then the if (lar) free(lar); should change IMHO. It's confusing.
Oh wow - I didn't even notice that. Yeah, it is confusing. Sorry about that.
Is lar->fd initialized to -1 btw?
lar->fd is the result of the open() - which will either be a -1 for errors or >= for a legit file descriptor.
The file can be corrupt, I just want to avoid making it 4G-256k.
I would be happy if lar exits with an error if the file size has changed between _open_lar() and _close_lar().
I'll do that - thats an easy check and it will probably save our bacon. Then you can tell me "I told you so!".
I'd prefer having that negation in the caller. Or at the very least doxygen comment it.
Yes. Much doxygen is in my future.
Thanks. I'm updating the code in real time, but suspect that we still have a bit more discussion before the next revision goes out.. :)
Jordan