lkml.org 
[lkml]   [2014]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2014-05-19 10:41    [W:0.355 / U:1.664 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site