Thanks for reviewing the patch Dan. >> TODO: re-write probe error handling Will submit a series of patches in upcoming days, based on the steps you provided. On Mon, Jan 24, 2022 at 12:19 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Sun, Jan 23, 2022 at 11:47:35PM +0530, Vihas Mak wrote: > > rtw_init_netdev_name() calls dev_alloc_name() which allocates the name > > for the device as per the given name format. > > The name format is specified by the module parameter "ifname". > > It returns a negative err code if the format is invalid. Handle this > > error appropriately. > > Cancel the timers ininitliazed by rtw_init_drv_sw() before calling > > rtw_free_drv_sw() and then proceed to free the adapter. > > > > Also, if register_netdev() fails then goto free_drv_sw instead of > > goto handle_dualmac. > > > > Signed-off-by: Vihas Mak <makvihas@xxxxxxxxx> > > --- > > v1->v2: > > free the adapter and netdev instead of warning the user about > > allocation failure. > > Thanks! > > Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > Of course, all this code is staging code and terrible. This function > is needlessly difficult to read/review. > > TODO: re-write probe error handling > > Step 1: Keep the success path and error path separate. > > - status = _SUCCESS; > + return padapter; > > Step 2: Eliminate do-nothing-gotos. s/goto exit/return NULL/ > > Step 3: Delete the vfree(pnpi->priv); from rtw_free_netdev() and call > vfree(pnpi->priv); from probe and rtw_usb_if1_deinit() instead. > Avoid a layering violation. > > Step 4: Every allocation function needs a matching free function. Move > the rtw_cancel_all_timer() into the rtw_free_drv_sw() function. > Open coding it is a layering violation. > > Step 5: Get rid of the rtw_handle_dualmac() function. It has a bad > name and a global variable. What is the point of this function? > > But that stuff is for later patches. > > regards, > dan carpenter > -- Thanks, Vihas
