Messages in this thread | | | Date | Mon, 19 May 2014 11:02:24 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH V3] staging: dgap: implement error handling in dgap_tty_register() |
| |
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;
> + > +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.
> +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
| |