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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/