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

From: Dan Carpenter
Date: Mon May 19 2014 - 04:02:54 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/