[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH v4 11/40] hw/xen: Use the IEC bin
From: |
Richard Henderson |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH v4 11/40] hw/xen: Use the IEC binary prefix definitions |
Date: |
Tue, 12 Jun 2018 11:10:19 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/12/2018 11:04 AM, Eric Blake wrote:
> On 06/12/2018 03:51 PM, Richard Henderson wrote:
>> On 06/10/2018 03:14 PM, Philippe Mathieu-Daudé wrote:
>>> xen_pv_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename
>>> \"%s\","
>>> - " size %" PRId64 " (%" PRId64 " MB)\n",
>>> + " size %" PRId64 " (%llu MB)\n",
>>> blkdev->type, blkdev->fileproto, blkdev->filename,
>>> - blkdev->file_size, blkdev->file_size >> 20);
>>> + blkdev->file_size, blkdev->file_size / MiB);
>>
>> Having to change printf markup is exactly why you shouldn't use ULL in MiB.
>
> Conversely, M_BYTE was already ULL, so if you don't use it in MiB, you'll have
> to change other printf markup where you were changing those uses.
>
> One benefit of using the widest possible type: we avoid risk of silent
> truncation. Potential downsides: wasted processing time (when 32 bits was
> sufficient), and compilers might start warning when we narrow a 64-bit value
> into a 32-bit variable (but I think we already ignore that).
>
> One benefit of using the natural type that holds the value: use of 64-bit math
> is explicit based on the type of what else is being multiplied by the macro.
> Potential downside: 32*32 assigned to a 64-bit result may be botched (but
> hopefully Coverity will flag it).
>
> So there's tradeoffs either way, and you at least need to document in your
> commit messages what auditing you have done that any type changes introduced
> by
> your changes are safe.
I'm more concerned about unnecessary or unintended signed vs unsigned changes
than I am about width. But if we're going to force a 64-bit type, use
(int64_t)1 not 1LL. That way the type will match the existing PRId64 printf
markup.
r~
- Re: [Qemu-trivial] [PATCH v4 05/40] hw: Use IEC binary prefix definitions from "qemu/units.h", (continued)
- [Qemu-trivial] [PATCH v4 06/40] hw: Directly use "qemu/units.h" instead of "qemu/cutils.h", Philippe Mathieu-Daudé, 2018/06/10
- [Qemu-trivial] [PATCH v4 07/40] hw/ivshmem: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
- [Qemu-trivial] [PATCH v4 08/40] hw/ipack: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
- [Qemu-trivial] [PATCH v4 09/40] hw/scsi: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
- [Qemu-trivial] [PATCH v4 10/40] hw/smbios: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
- [Qemu-trivial] [PATCH v4 11/40] hw/xen: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 12/40] hw/tpm: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 13/40] hw/block: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 15/40] hw/misc: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 16/40] hw/riscv: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 14/40] hw/display: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10
[Qemu-trivial] [PATCH v4 17/40] hw/m68k: Use the IEC binary prefix definitions, Philippe Mathieu-Daudé, 2018/06/10