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

[Andreas Färber]

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...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg