Re: [PATCH V3] staging: dgap: implement error handling in dgap_tty_register()

Dan Carpenter
2014-05-19T08:02:49Z

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