[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] bitops.h: Add field32() and field64() functions

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH] bitops.h: Add field32() and field64() functions to extract bitfields
Date: Thu, 28 Jun 2012 07:58:52 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux)

Blue Swirl <address@hidden> writes:

> On Wed, Jun 27, 2012 at 6:01 AM, Markus Armbruster <address@hidden> wrote:
>> Blue Swirl <address@hidden> writes:
>>> On Tue, Jun 26, 2012 at 6:41 PM, Peter Maydell <address@hidden> wrote:
>>>> On 26 June 2012 19:25, Blue Swirl <address@hidden> wrote:
>>>>> On Tue, Jun 26, 2012 at 6:11 PM, Peter Maydell <address@hidden> wrote:
>>>>>> On 26 June 2012 18:58, Blue Swirl <address@hidden> wrote:
>>>>>>> On Mon, Jun 25, 2012 at 7:38 PM, Peter Maydell <address@hidden> wrote:
>>>>>>>> +static inline uint64_t field64(uint64_t value, int start, int length)
>>>>>>> start and length could be unsigned.
>>>>>> They could be, but is there any reason why they should be?
>>>>>> set_bit(), clear_bit() etc use 'int' for bit numbers, so this
>>>>>> is consistent with that.
>>>>> Negative shifts don't work, the line with assert() would get shorter
>>>>> and simpler and I like unsigned values.
>>>> I don't like using unsigned for numbers that merely happen to always
>>>> be positive (as opposed to actually requiring unsigned arithmetic)[*],
>>>> so I think I'll stick with being consistent with the existing bitops
>>>> functions, thanks :-)
>> Consistency is a strong argument.
> Yes, but if using unsigned ints for the all bit ops makes sense, they
> all should be converted. This case could start using unsigned ints
> before that. It's the same as CODING_STYLE.
>>> Using unsigned types also produces better code in some cases. There
>> Better code is an argument only if the effect can be demonstrated.
> I don't know even for which compilers or CPUs this is true so it's
> unlikely I could demonstrate it. However, googling finds a few
> articles in defense of this.

Hearsay.  Your honor, I rest my case :)

>>> are also operations which do not work well with signed integers (%,
>>> >>).
>> Operator >> is applicable here.  It works exactly as well for negative
>> right operand as it does for large positive right operand: undefined
>> behavior.
> Score one for the unsigned which avoids the negative case.

No, my point is: with unsigned the negative case turns into the large
positive case, which is exactly as bad.

>>>> [*] the classic example of where that kind of thing can trip you up
>>>> is the way it complicates the termination condition on a "for (i = N;
>>>> i >= 0; i--)" decreasing loop.
>> Yup.  There are more, e.g. fun with unwanted truncation or sign
>> extension when mixing different integer types.
> Yes, mixing signedness is not fun.
>>  Stick to int unless you
>> have a compelling reason.
> I've seen exactly opposite guidance: use unsigned whenever possible.

Leads to much dangerous mixing of different integer types, and ugly
casts all over the place.

> In this case, using signed values for bit numbers is never useful. The
> bit numbers simply are not meaningful if negative, so unsigned values
> are more intuitive. The types could be even smaller, like uint8_t.

Narrow parameter types only hide programming errors here, because
arguments get silently truncated before they can reach the protective

reply via email to

[Prev in Thread] Current Thread [Next in Thread]