Re: [PATCH] tools/libxc: Fix error checking for xc_get_{cpu, node}map_size() callers

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2983 bytes --]

On ven, 2013-12-13 at 00:35 +0000, Andrew Cooper wrote:
> On 12/12/2013 23:59, Dario Faggioli wrote:
> > Well, yes, but under what circumstances Xen would do such a thing? As
> > far as I can see, max_node_id is just 'MAX_NUMNODES-1'. max_cpu_id is
> > 'nr_cpu_ids-1', nr_cpu_ids is '__read_mostly nr_cpu_ids = NR_CPUS'.
> >
> > I may be wrong, but it looks to me that either both MAX_NUMNODES and
> > NR_CPUS (and nr_cpu_ids+1 too, if it changes) are > 0, or the system
> > would be experiencing way bigger issues than misdimensioning a bitmap.
> >
> > What I mean is, if we are there checking, we at least have one node and
> > one cpu. In which case, either the call failed and returned <0, or it
> > succeeded, and returned >0.
> >
> > What am I missing?
> 
> I didn't wish to imply that I expected Xen to return -1 for either
> case.  Stuff would indeed be very broken if this were the case.
> 
Right..

> As the argument is over the difference between "< 0" and "<= 0",
> defensive coding would have the "<= 0" check even if Xen is a trusted
> source of information.
> 
Indeed. On the other hand, having the "<=" could confuse people looking
at the code, tricking them into thinking "why the '<='? How is it
possible for this to be '=0'?". One could stick comments there, but it
would probably make it too chatty.

Anyway, especially after all this, I'm really fine with either. Even if
'arguing' is the real big fun in Open Source, I guess it's the time to
let this go, and hear what the "bosses" want and do that. :-P

> However,
> 
> calloc(1, 0) (just like malloc(0) ) can give you a valid pointer to a
> buffer you cannot use, and indeed glibc does give you a real buffer of
> length 0.
> 
> This very dangerous, as traditional thinking says "if I have a non-null
> pointer in my hands, its good".  As soon as you dereference this
> pointer, you have undefined behaviour.
> 
Agreed.

> From what I understand from comp.lang.c, the only reason this is in the
> spec (rather than being a very strict "malloc(0) => NULL") is that
> implementations at the time of standardisation already had this behaviour.
> 
Yeah... In spite of anyone that thinks standards should promote best
practises, rather than blessing already existing and bad ones! :-P

> Whatever the reason for these quirks existing, they are best avoided
> whenever possible.
> 
Sure!

> I too will end up deferring to a specific judgement from a tools
> maintainer.  I am just taking this opportunity to justify why I chose
> "<= 0" in all cases rather than "< 0" (which certainly did get considered).
> 
Right. Thanks for the explanation, and for the arguing. :-)

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel