Re: [Xen-devel] [PATCH 13/17 v5] xen/arm: vpl011: Modify xenconsole to support multiple consoles

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jul 07, 2017 at 07:49:32PM +0530, Bhupinder Thakur wrote:
> Hi Wei,
> 
> On 7 July 2017 at 19:30, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> > On Fri, Jul 07, 2017 at 07:22:14PM +0530, Bhupinder Thakur wrote:
> >> >> -static struct domain *create_domain(int domid)
> >> >> +static int console_init(struct console *con, struct domain *dom, void 
> >> >> **data)
> >> >>  {
> >> >> -     struct domain *dom;
> >> >>       char *s;
> >> >> +     int err = -1;
> >> >>       struct timespec ts;
> >> >> -     struct console *con;
> >> >> +     struct console_data **con_data = (struct console_data **)data;
> >> >> +     char *xsname;
> >> >>
> >> >>       if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
> >> >>               dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
> >> >>                     __FILE__, __FUNCTION__, __LINE__);
> >> >> -             return NULL;
> >> >> +             return err;
> >> >> +     }
> >> >> +
> >> >
> >> > There is a danger that you return at this point, the cleanup path in
> >> > caller will free garbage.
> >> >
> >> > I suggest you at least initialise all pointers to NULL at the beginning.
> >> >
> >> I am checking that the pointer is not null before freeing them.
> >>
> >
> > I'm not sure what your reply means.
> >
> > Without initialising the pointers to NULL you can have garbage in your
> > pointer (currently only xspath) -- you end up freeing the garbage
> > pointer. That's why I made the suggestion in the first place. Checking
> > NULL isn't going to help that.
> 
> I believe when domain structure is allocated it is initialized with 0
> (as calloc is used for allocation) so all fields
> in the domain and console structures should be NULL.
> 

OK in that case it is fine. Misread part of your patch.

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