[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] [qemu] xen_be_init under stubdom
>> Do nothing in xen_be_init under stubdom plus a minor inconsequential cleanup. > ... >> - goto cleanup; >> + return; > ... >> -cleanup: >> qemu_free(vec); >> } > > I don't think this is a helpful change. I did call it inconsequential and wouldn't have sent it if it didn't happen to be with a one liner that had functional impact that I wanted changed :) > There is nothing wrong with > calling qemu_free(0) and IMO in general functions that "goto cleanup" > are to be preferred to ones that "return". > You would rather check for null pointer and then go to cleanup code whose sole purpose in this case is to free that pointer and all this for free to realize that it has nothing to free and then return back! This as opposed to simply return when it is null! I understand how going to cleanup makes a lot of sense when there is at least some amount of potential cleanup to do. In this case, clean up does nothing more than free that pointer! Anyway, I am not particularly worried if this doesn't make it. > Furthermore, even if this patch were good, it's not a bugfix so is not > acceptable at this stage of the release. > No, it does get rid of an issue I encounter. I was running into data corruption as this code path was taken with stubdom in my configuration and it was a pain to debug as with these kinds of corruption issues the problem showed up elsewhere. Kamala >> @@ -646,6 +645,10 @@ static void xen_be_evtchn_event(void *opaque) >> >> int xen_be_init(void) >> { >> +#ifdef CONFIG_STUBDOM >> + return 0; >> +#endif > > I don't understand this at all. Why should stubdom not be able to > make pv backends if it wants to ? I agree that it probably doesn't > want to but if something iswrongly causing it to then the right fix is > to make it not do so. > > Ian. > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |