From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] tools/libxc: Fix error checking for xc_get_{cpu, node}map_size() callers Date: Fri, 13 Dec 2013 00:59:18 +0100 Message-ID: <1386892758.5488.140.camel@Solace> References: <1386776862-19921-1-git-send-email-andrew.cooper3@citrix.com> <1386858242.18946.1.camel@kazak.uk.xensource.com> <1386860178.5488.113.camel@Solace> <52AA252B.2070109@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8061024335384538485==" Return-path: In-Reply-To: <52AA252B.2070109@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: George Dunlap , Ian Jackson , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org --===============8061024335384538485== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-ND7Uy2l+2KxVQZIvWTIa" --=-ND7Uy2l+2KxVQZIvWTIa Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On gio, 2013-12-12 at 21:05 +0000, Andrew Cooper wrote: > On 12/12/2013 14:56, Dario Faggioli wrote: > > Yep, I confirm that, after that changeset, neither > > xc_get_max_{cpus,nodes}() nor xc_get_{cpu,node}map_size() return 0 as a= n > > error anymore. >=20 > Zero might not be "the error condition" any more, but it is certainly an > error from any of these functions (and possible as > xc_get_max_{cpus,nodes}() is capable of returning 0 if Xen hands back -1 > for physinfo.max_{cpu,node}_id) >=20 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 =3D 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? > xc_{cpu/node}map_alloc() must strictly still be "<=3D 0" checks to avoid > the issue where calloc(1, 0) returns a non-NULL pointer. >=20 Here `man calloc' says, among other things: "The memory is set to zero. If nmemb or size is 0, then calloc() returns either NULL, or a unique pointer value that can later be successfully passed to free()." Was it that what you were referring to? > Currently, I am of the opinion that the patch is better as is, than > changing some of the checks to being strictly "< 0" >=20 Given the first part of this reply (if I'm not mistaken in there) I'd prefer the other way round. I.e., '< 0' whenever it makes sense and, if it's an actual issue, '<=3D 0' in xc_{cpu/node}map_alloc(), perhaps with a comment, saying that the '<=3D' is there to prevent calloc madness. :-) That being said, I'm happy with whatever solution a tool maintainer likes better. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-ND7Uy2l+2KxVQZIvWTIa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) iEYEABECAAYFAlKqTdYACgkQk4XaBE3IOsT7LgCfcPIGtrPRL4QYQegyYZiq4JjD NYkAoIcVEfzslKlyYoAIlGVVyVbVqrI7 =Cs8B -----END PGP SIGNATURE----- --=-ND7Uy2l+2KxVQZIvWTIa-- --===============8061024335384538485== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8061024335384538485==--