[PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Tiejun Chen]

This shouldn't ever be negative.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 xen/include/asm-x86/e820.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/asm-x86/e820.h b/xen/include/asm-x86/e820.h
index 8418fc4..0bd5c2f 100644
--- a/xen/include/asm-x86/e820.h
+++ b/xen/include/asm-x86/e820.h
@@ -19,7 +19,7 @@ struct __packed e820entry {
 #define E820MAX	128
 
 struct e820map {
-    int nr_map;
+    unsigned int nr_map;
     struct e820entry map[E820MAX];
 };
 
-- 
1.9.1

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Jan Beulich]

>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
> This shouldn't ever be negative.

This means nothing for the actual change, and makes me imply you did
the change without checking it actually _is_ never negative in any of
the uses.

Also please cut down your patch title prefixes (not just here) to
something reasonable.

Jan

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Chen, Tiejun]

On 2014/8/8 15:15, Jan Beulich wrote:
>>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
>> This shouldn't ever be negative.
>
> This means nothing for the actual change, and makes me imply you did
> the change without checking it actually _is_ never negative in any of
> the uses.
>
> Also please cut down your patch title prefixes (not just here) to
> something reasonable.
>

What about this?

xen:e820map: just convert nr_map with unsigned int

nr_map is used to represent the amount of e820entry in e820 so it
shouldn't be negative.

Thanks
Tiejun

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Jan Beulich]

>>> On 08.08.14 at 09:26, <tiejun.chen@intel.com> wrote:
> On 2014/8/8 15:15, Jan Beulich wrote:
>>>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
>>> This shouldn't ever be negative.
>>
>> This means nothing for the actual change, and makes me imply you did
>> the change without checking it actually _is_ never negative in any of
>> the uses.
>>
>> Also please cut down your patch title prefixes (not just here) to
>> something reasonable.
>>
> 
> What about this?
> 
> xen:e820map: just convert nr_map with unsigned int

x86: make struct e820map's nr_map unsigned

> nr_map is used to represent the amount of e820entry in e820 so it
> shouldn't be negative.

You still say "shouldn't" instead of "isn't", making me still imply you
didn't verify that fact. I.e. I'd be fine with a suitable variant of the
earlier more terse description as long as it says what it needs to say.

Jan

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Chen, Tiejun]

On 2014/8/8 15:38, Jan Beulich wrote:
>>>> On 08.08.14 at 09:26, <tiejun.chen@intel.com> wrote:
>> On 2014/8/8 15:15, Jan Beulich wrote:
>>>>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
>>>> This shouldn't ever be negative.
>>>
>>> This means nothing for the actual change, and makes me imply you did
>>> the change without checking it actually _is_ never negative in any of
>>> the uses.
>>>
>>> Also please cut down your patch title prefixes (not just here) to
>>> something reasonable.
>>>
>>
>> What about this?
>>
>> xen:e820map: just convert nr_map with unsigned int
>
> x86: make struct e820map's nr_map unsigned

Okay.

>
>> nr_map is used to represent the amount of e820entry in e820 so it
>> shouldn't be negative.
>
> You still say "shouldn't" instead of "isn't", making me still imply you
> didn't verify that fact. I.e. I'd be fine with a suitable variant of the

Maybe I'm poor to describe that main point here.

x86: make struct e820map's nr_map unsigned

nr_map is used to represent the amount of e820entry in e820 so its
not always negative.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

Thanks
Tiejun

> earlier more terse description as long as it says what it needs to say.
>
> Jan
>
>
>

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Jan Beulich]

>>> On 08.08.14 at 09:54, <tiejun.chen@intel.com> wrote:
> On 2014/8/8 15:38, Jan Beulich wrote:
>>>>> On 08.08.14 at 09:26, <tiejun.chen@intel.com> wrote:
>>> On 2014/8/8 15:15, Jan Beulich wrote:
>>>>>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
>>>>> This shouldn't ever be negative.
>>>>
>>>> This means nothing for the actual change, and makes me imply you did
>>>> the change without checking it actually _is_ never negative in any of
>>>> the uses.
>>>>
>>>> Also please cut down your patch title prefixes (not just here) to
>>>> something reasonable.
>>>>
>>>
>>> What about this?
>>>
>>> xen:e820map: just convert nr_map with unsigned int
>>
>> x86: make struct e820map's nr_map unsigned
> 
> Okay.
> 
>>
>>> nr_map is used to represent the amount of e820entry in e820 so it
>>> shouldn't be negative.
>>
>> You still say "shouldn't" instead of "isn't", making me still imply you
>> didn't verify that fact. I.e. I'd be fine with a suitable variant of the
> 
> Maybe I'm poor to describe that main point here.
> 
> x86: make struct e820map's nr_map unsigned
> 
> nr_map is used to represent the amount of e820entry in e820 so its
> not always negative.

"not always negative"?

Based on your initial one, "This is never negative" would be fine,
_provided you verified_ this to be the case.

Jan

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Chen, Tiejun]

On 2014/8/8 16:49, Jan Beulich wrote:
>>>> On 08.08.14 at 09:54, <tiejun.chen@intel.com> wrote:
>> On 2014/8/8 15:38, Jan Beulich wrote:
>>>>>> On 08.08.14 at 09:26, <tiejun.chen@intel.com> wrote:
>>>> On 2014/8/8 15:15, Jan Beulich wrote:
>>>>>>>> On 08.08.14 at 09:00, <tiejun.chen@intel.com> wrote:
>>>>>> This shouldn't ever be negative.
>>>>>
>>>>> This means nothing for the actual change, and makes me imply you did
>>>>> the change without checking it actually _is_ never negative in any of
>>>>> the uses.
>>>>>
>>>>> Also please cut down your patch title prefixes (not just here) to
>>>>> something reasonable.
>>>>>
>>>>
>>>> What about this?
>>>>
>>>> xen:e820map: just convert nr_map with unsigned int
>>>
>>> x86: make struct e820map's nr_map unsigned
>>
>> Okay.
>>
>>>
>>>> nr_map is used to represent the amount of e820entry in e820 so it
>>>> shouldn't be negative.
>>>
>>> You still say "shouldn't" instead of "isn't", making me still imply you
>>> didn't verify that fact. I.e. I'd be fine with a suitable variant of the
>>
>> Maybe I'm poor to describe that main point here.
>>
>> x86: make struct e820map's nr_map unsigned
>>
>> nr_map is used to represent the amount of e820entry in e820 so its
>> not always negative.
>
> "not always negative"?
>
> Based on your initial one, "This is never negative" would be fine,

Will be fixed.

> _provided you verified_ this to be the case.
>

Sorry, are you saying I need to add something in patch description,

x86: make struct e820map's nr_map unsigned

nr_map is used to represent the amount of e820entry in e820 so
this is never negative.

Note just boot one VM to confirm this is really fine.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

Thanks
Tiejun

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Jan Beulich]

>>> On 08.08.14 at 11:06, <tiejun.chen@intel.com> wrote:
> On 2014/8/8 16:49, Jan Beulich wrote:
>> _provided you verified_ this to be the case.
>>
> 
> Sorry, are you saying I need to add something in patch description,

No (at least not normally, but with it taking so long for you to
understand what is needed, I guess we need you to state this
explicitly) - all I want is that you state the right thing, and what
you state is true.

> x86: make struct e820map's nr_map unsigned
> 
> nr_map is used to represent the amount of e820entry in e820 so
> this is never negative.
> 
> Note just boot one VM to confirm this is really fine.

This - again - is no proof of anything.

Jan

Re: [PATCH 1/1] xen:include:asm-x86:e820: just covert nr_map with unsigned int

[Chen, Tiejun]

On 2014/8/8 17:18, Jan Beulich wrote:
>>>> On 08.08.14 at 11:06, <tiejun.chen@intel.com> wrote:
>> On 2014/8/8 16:49, Jan Beulich wrote:
>>> _provided you verified_ this to be the case.
>>>
>>
>> Sorry, are you saying I need to add something in patch description,
>
> No (at least not normally, but with it taking so long for you to
> understand what is needed, I guess we need you to state this
> explicitly) - all I want is that you state the right thing, and what
> you state is true.
>

Understand and thanks so much.

Tiejun