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

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

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.

Regards,
Bhupinder

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