2014-05-19 18:39 GMT+09:00 DaeSeok Youn <daeseok.youn@xxxxxxxxx>: > 2014-05-19 17:02 GMT+09:00 Dan Carpenter <dan.carpenter@xxxxxxxxxx>: >> Nice, but it needs a couple style improvements below. >> >> On Mon, May 19, 2014 at 11:10:30AM +0900, Daeseok Youn wrote: >>> + brd->dgap_major_serial_registered = TRUE; >>> + dgap_boards_by_major[brd->serial_driver->major] = brd; >>> + brd->dgap_serial_major = brd->serial_driver->major; >>> + >>> brd->dgap_major_transparent_print_registered = TRUE; >>> dgap_boards_by_major[brd->print_driver->major] = brd; >>> brd->dgap_transparent_print_major = brd->print_driver->major; >>> >>> return rc; >> >> return 0; > OK. I will remove "int rc = 0" line and change "return rc" to "return 0" Oh.. just remove the initialization to zero. > >> >>> + >>> +unregister_serial_drv: >>> + tty_unregister_driver(brd->serial_driver); >>> +free_print_ttys: >>> + kfree(brd->print_driver->ttys); >>> + brd->print_driver->ttys = NULL; >> >> This label isn't needed. Just goto free_print_drv, because that will >> free the brd->print_driver->ttys in destruct_tty_driver(). >> I do like how you noticed the double free and avoided it by setting >> brd->serial_driver->ttys to NULL, so your patch doesn't introduce a >> double free bug. > Yes, just goto free_print_drv. > > Thanks for review. > > Regards, > Daeseok Youn >> >>> +free_print_drv: >>> + put_tty_driver(brd->print_driver); >>> +free_serial_ttys: >>> + kfree(brd->serial_driver->ttys); >>> + brd->serial_driver->ttys = NULL; >> >> Same for this. >> >>> +free_serial_drv: >>> + put_tty_driver(brd->serial_driver); >>> + >>> + return rc; >> >> regards, >> dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
