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" > >> + >> +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
