On Mon, Sep 23, 2013 at 06:47:16PM -0400, Lidza Louina wrote:
> This patch removes these warnings:
> potential null dereference 'brd->SerialDriver'. (alloc_tty_driver returns null)
> potential null dereference 'brd->PrintDriver'. (alloc_tty_driver returns null)
>
> This warning popped up because there wasn't a check
> to make sure that the serial and print drivers were
> allocated and not null before being initialized. This
> patch adds that check.
>
> Signed-off-by: Lidza Louina <lidza.louina@xxxxxxxxx>
> ---
> drivers/staging/dgap/dgap_tty.c | 103 +++++++++++++++++++++-------------------
> 1 file changed, 54 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/staging/dgap/dgap_tty.c b/drivers/staging/dgap/dgap_tty.c
> index 924e2bf..8f0a824 100644
> --- a/drivers/staging/dgap/dgap_tty.c
> +++ b/drivers/staging/dgap/dgap_tty.c
> @@ -220,66 +220,71 @@ int dgap_tty_register(struct board_t *brd)
> DPR_INIT(("tty_register start"));
>
> brd->SerialDriver = alloc_tty_driver(MAXPORTS);
> + if(brd->SerialDriver){
Don't do it that way, flip it around.
brd->SerialDriver = alloc_tty_driver(MAXPORTS);
if (!brd->SerialDriver)
return -ENOMEM;
snprintf(brd->SerialName, MAXTTYNAMELEN, "tty_dgap_%d_", brd->boardnum);
brd->SerialDriver->name = brd->SerialName;
That way has fewer indents.
When you're writing code, you want it to read in a straight line going
down. You don't want long if statement blocks or spaghetti code. If
you hit an error deal with it immediately and continue with the story in
a straight line going down.
> +
> + /* The kernel wants space to store pointers to tty_structs */
> + brd->SerialDriver->ttys = dgap_driver_kzmalloc(MAXPORTS * sizeof(struct tty_struct *), GFP_KERNEL);
> + if (!brd->SerialDriver->ttys)
> + return(-ENOMEM);
On this if statement it's a little bit more complicated because you have
to free the memory you allocated earlier before returning. This one
should look like:
brd->SerialDriver->ttys = dgap_driver_kzmalloc(MAXPORTS * sizeof(struct tty_struct *), GFP_KERNEL);
if (!brd->SerialDriver->ttys) {
ret = -ENOMEM;
goto err_put_driver;
}
tty_set_operations(brd->SerialDriver, &dgap_tty_ops);
At the end of the function there will be something like:
return 0;
err_release_foo:
release_foo();
err_free_something:
free_some_more_stuff();
err_put_driver:
put_tty_driver(brd->SerialDriver);
return ret;
}
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
