On Mon, Oct 29, 2012 at 07:23:47PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:15PM -0700, George Zhang wrote:
> > +static int __init vmci_core_init(void)
> > +{
> > + int result;
> > +
> > + result = vmci_ctx_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIContext (result=%d).\n",
> > + result);
>
> If you are going to use pr_* functions, it's usually a good idea to
> define pr_fmt in a consistant way so that it shows up the same across
> all of your .c files. See other drivers for examples of how to do this
> properly.
pr_fmt() is defined in drivers/misc/vmw_vmci/vmci_common_int.h
>
> > + return -EINVAL;
> > + }
> > +
> > + result = vmci_datagram_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIDatagram (result=%d).\n",
> > + result);
> > + return -EINVAL;
> > + }
> > +
> > + result = vmci_event_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIEvent (result=%d).\n", result);
> > + return -EINVAL;
> > + }
>
> You don't free and unwind things properly if things go wrong here, why
> not?
We do, the above calls do not need to be cleaned up.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit vmci_core_exit(void)
> > +{
> > + vmci_event_exit();
> > +}
> > +
> > +static int __init vmci_drv_init(void)
> > +{
> > + int error;
> > +
> > + error = vmci_core_init();
> > + if (error)
> > + return error;
> > +
> > + if (!vmci_disable_guest) {
> > + error = vmci_guest_init();
> > + if (error) {
> > + pr_warn("Failed to initialize guest personality
> > (err=%d).\n",
> > + error);
> > + } else {
> > + vmci_guest_personality_initialized = true;
> > + pr_info("Guest personality initialized and is %s\n",
> > + vmci_guest_code_active() ?
> > + "active" : "inactive");
> > + }
> > + }
> > +
> > + if (!vmci_disable_host) {
> > + error = vmci_host_init();
> > + if (error) {
> > + pr_warn("Unable to initialize host personality
> > (err=%d).\n",
> > + error);
> > + } else {
> > + vmci_host_personality_initialized = true;
> > + pr_info("Initialized host personality\n");
> > + }
> > + }
> > +
> > + if (!vmci_guest_personality_initialized &&
> > + !vmci_host_personality_initialized) {
> > + vmci_core_exit();
> > + return -ENODEV;
> > + }
> > +
> > + pr_debug("Module is initialized\n");
>
> Really? You need this message still?
These are debug messages so do not show up in logs by default.
>
> > + return 0;
> > +}
> > +module_init(vmci_drv_init);
> > +
> > +static void __exit vmci_drv_exit(void)
> > +{
> > + if (vmci_guest_personality_initialized)
> > + vmci_guest_exit();
> > +
> > + if (vmci_host_personality_initialized)
> > + vmci_host_exit();
> > +
> > + vmci_core_exit();
> > + pr_debug("Module is unloaded\n");
>
> Same here, is this really needed?
>
Still debug so no littering in logs.
Thanks,
Dmitry
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
