Mailing List Archive: Re: [PATCH 23 of 32 RFC] libxl: use libxl_*

Re: [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too

Jan 16, 2012, 8:15 AM

Post #1 of 5 (102 views)

On Mon, 2012-01-16 at 16:09 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too"):
> > libxl: use libxl_*_init internally too
> ...
> > + ret = libxl_device_vfb_init(CTX, vfb);
> > + if (ret) return ret;
>
> Can we make these init functions infallible ? (I haven't read
> their code yet...)

I think the reason is that sometimes they might allocate memory. Many of
them don't (I don't know about vfb in particular) and I don't know
whether we prefer consistency in similar functions or avoiding pointless
return values (I prefer the former so as to avoid needing to change the
public API in the future when we find that we have introduced a way for
a function to fail).

Possibly in the new world order (where init ~= memset and setdefaults
does thework) this will no longer be the case, but then the same will
become true of the setdefaults function.

Ian.

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

Re: [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too [ In reply to ]

Ian.Jackson at eu

Jan 16, 2012, 8:09 AM

Post #2 of 5 (100 views)

Permalink

Ian Campbell writes ("[Xen-devel] [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too"):
> libxl: use libxl_*_init internally too
...
> + ret = libxl_device_vfb_init(CTX, vfb);
> + if (ret) return ret;

Can we make these init functions infallible ? (I haven't read
their code yet...)

Ian.

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

Re: [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too [ In reply to ]

Ian.Jackson at eu

Jan 16, 2012, 8:16 AM

Post #3 of 5 (101 views)

Permalink

Ian Campbell writes ("Re: [Xen-devel] [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too"):
> I think the reason is that sometimes they might allocate memory. Many of
> them don't (I don't know about vfb in particular) and I don't know
> whether we prefer consistency in similar functions or avoiding pointless
> return values (I prefer the former so as to avoid needing to change the
> public API in the future when we find that we have introduced a way for
> a function to fail).

Why might they need to allocate memory ?

> Possibly in the new world order (where init ~= memset and setdefaults
> does thework) this will no longer be the case, but then the same will
> become true of the setdefaults function.

Yes. I think it's OK for setdefaults to be able to fail.

Having infallible init functions is a real boon because it makes the
caller's memory management much easier, because they can always safely
dispose.

Ian.

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

Re: [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too [ In reply to ]

Ian.Campbell at citrix

Jan 16, 2012, 8:23 AM

Post #4 of 5 (98 views)

Permalink

On Mon, 2012-01-16 at 16:16 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too"):
> > I think the reason is that sometimes they might allocate memory. Many of
> > them don't (I don't know about vfb in particular) and I don't know
> > whether we prefer consistency in similar functions or avoiding pointless
> > return values (I prefer the former so as to avoid needing to change the
> > public API in the future when we find that we have introduced a way for
> > a function to fail).
>
> Why might they need to allocate memory ?

e.g. vif->model = strdup("rtl8139").

> > Possibly in the new world order (where init ~= memset and setdefaults
> > does thework) this will no longer be the case, but then the same will
> > become true of the setdefaults function.
>
> Yes. I think it's OK for setdefaults to be able to fail.
>
> Having infallible init functions is a real boon because it makes the
> caller's memory management much easier, because they can always safely
> dispose.

I think my series has made more *_init functions have no possibility of
failure but I don't think it is all of them.

Ian.

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

Re: [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too [ In reply to ]

Ian.Jackson at eu

Jan 16, 2012, 8:25 AM

Post #5 of 5 (101 views)

Permalink

Ian Campbell writes ("Re: [Xen-devel] [PATCH 23 of 32 RFC] libxl: use libxl_*_init internally too"):
> On Mon, 2012-01-16 at 16:16 +0000, Ian Jackson wrote:
> > Why might they need to allocate memory ?
>
> e.g. vif->model = strdup("rtl8139").

That should be done in setdefault, not in init. If for no other
reason than doing it in init means the application has to free again
this string which we pointless allocated.

> > Having infallible init functions is a real boon because it makes the
> > caller's memory management much easier, because they can always safely
> > dispose.
>
> I think my series has made more *_init functions have no possibility of
> failure but I don't think it is all of them.

Are there any that are left that are fallible ?

Ian.

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