Re: [Xen-devel] libxl_cdrom_insert

[Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>]

On Sat, Jan 15, 2011 at 10:03 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Fri, 2011-01-14 at 22:37 +0000, Kamala Narasimhan wrote:
>> Do we know the rationale behind setting disk->physpath to "" in the
>> below code in libxl?  Hopefully there is more to it than to avoid a
>> potential null pointer crash at a later point :)
>
> I think it might indicate an empty CDROM drive in the HVM case. Arguably
> a PHYSTYPE_EMPTY/NONE or some such concept might have been clearer. "hg
> annotate" should lead you to the original commit.
>

In that case, if we want to keep "", per Gianni Tedesco we would need
the below fix as the string would get freed later -

diff -r ce208811f540 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu Jan 13 01:26:44 2011 +0000
+++ b/tools/libxl/libxl.c       Fri Jan 14 17:39:16 2011 -0500
@@ -1676,7 +1676,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     int ret = ERROR_FAIL;

     if (!disk->physpath) {
-        disk->physpath = "";
+        disk->physpath = strdup("");
         disk->phystype = PHYSTYPE_PHY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);

Gianni might have to sign off on it or at least ack it but here is
mine if that is needed too -

Signed-off-by: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxxx>

Kamala

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel