Re: [Qemu-devel] [PATCH] use an unsigned long for the max_sz parameter in load_image_targphys

[Alexander Graf]

On 09.03.2012, at 19:47, Andreas Färber wrote:

> Am 09.03.2012 18:11, schrieb Peter Maydell:
>> On 9 March 2012 14:28, Andreas Färber <address@hidden> wrote:
>>> No, please. We're describing sizes, not addresses. target_phys_addr_t
>>> thus is semantically wrong here. The RAM size is unsigned long IIRC (it
>>> is limited by the host's available memory). If you subtract something
>>> from a size it remains a size. I had therefore suggested size_t before.
>>> I expect sizeof(size_t) >= sizeof(unsigned long).
>> 
>> We're discussing target sizes. size_t might be smaller than
>> target_phys_addr_t, so it's also semantically wrong. We don't
>> have a target_size_t, though, and I think "use an address
>> related type for an offset" is less bad than "use a host
>> sized type for a guest sized value".
> 
> That is a moot point. There is no such thing as a "target size". The
> size is defined by the guest OS, not by the architecture. And it doesn't
> matter if the guest OS's size is defined larger than the host's because
> we process those files on the host and they must fit into host memory.
> 
> So unsigned long would be perfectly fine if ignoring oddball win64.
> 
>> Compare the way we use target_phys_addr_t for the offset arguments
>> to MemoryRegion read/write functions.
> 
> Nobody here has been arguing against using target_phys_addr_t for guest
> memory *offsets*.
> 
> Actually, the size (1, 2, 4) is an unsigned int there. :) Fine with me.
> 
> And due to very similar signed overflow issues with int64_t, 128-bit
> integer support was introduced as a workaround. ;)
> 
> 
> Anyway, POSIX:2008 says this about sys/types.h:
> 
> off_t
>       Used for file sizes.
> size_t
>       Used for sizes of objects.
> ssize_t
>       Used for a count of bytes or an error indication.
> 
> So off_t seems right for get_image_size() although a bit
> counter-intuitive. However later is said, "off_t shall be signed integer
> types". So on a 32-bit host that does not necessarily help for the case
> at hands. (Since get_image_size() gets its value as an off_t though, it
> doesn't matter there and improves the 64-bit host case.)
> 
> By comparison, "size_t shall be an unsigned integer type" and "The
> implementation shall support one or more programming environments in
> which the widths of [...] size_t, ssize_t, [...] are no greater than the
> width of type long".
> 
> So I still think that size_t is our best bet for potentially large sizes
> here. Comparison with off_t would cause warnings though...

You're on the wrong track here really :). Size, max_size and friends are all 
offsets into the guest's address space, so that's the type they should have. We 
don't care about host POSIX whatever semantics here - it's a question of how 
big can guest address space grow.

Imagine, your host can only do 32 bit file offsets. You want to emulate a 64bit 
guest though. To load an image, you say "load the image at offset x, max size 
<guest ram size - x>". If you use size_t, it would break for large ram guests, 
because the size really means semantically that you want to be able to load 
into address [x...ram_size] of the guest's physical memory range. If the host 
can actually load images that big is an orthogonal question.


Alex