On Fri, Sep 06, 2013 at 04:48:32PM -0400, Lidza Louina wrote:
> The arguments that are passed into sizeof were
> generic. This patch changes this by putting
> the actual item that we need a size of instead.
>
> For example:
> - kzalloc(sizeof(struct dgnc_board), GFP_KERNEL);
> + kzalloc(sizeof(brd), GFP_KERNEL);
>
> Signed-off-by: Lidza Louina <lidza.lou...@gmail.com>
> ---
> drivers/staging/dgnc/dgnc_driver.c | 4 ++--
> drivers/staging/dgnc/dgnc_mgmt.c | 2 +-
> drivers/staging/dgnc/dgnc_tty.c | 24 ++++++++++++------------
> 3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_driver.c
> b/drivers/staging/dgnc/dgnc_driver.c
> index 5b4d799..a1b24b5 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -487,14 +487,14 @@ static int dgnc_found_board(struct pci_dev *pdev, int
> id)
>
> /* get the board structure and prep it */
> brd = dgnc_Board[dgnc_NumBoards] =
> - kzalloc(sizeof(struct dgnc_board), GFP_KERNEL);
> + kzalloc(sizeof(brd), GFP_KERNEL);
Still not right.
sizeof(*brd);
I'm always creating test small test programs:
struct foo {
char buf[42];
};
int main(void)
{
struct foo *p;
printf("%ld %ld\n", sizeof(p), sizeof(*p));
return 0;
}
> if (!brd) {
> return -ENOMEM;
> }
>
> /* make a temporary message buffer for the boot messages */
> brd->msgbuf = brd->msgbuf_head =
> - kzalloc(sizeof(char) * 8192, GFP_KERNEL);
> + kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
> if (!brd->msgbuf) {
> kfree(brd);
> return -ENOMEM;
> diff --git a/drivers/staging/dgnc/dgnc_mgmt.c
> b/drivers/staging/dgnc/dgnc_mgmt.c
> index bb39f5d..354458c 100644
> --- a/drivers/staging/dgnc/dgnc_mgmt.c
> +++ b/drivers/staging/dgnc/dgnc_mgmt.c
> @@ -209,7 +209,7 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> uint board = 0;
> uint channel = 0;
>
> - if (copy_from_user(&ni, uarg, sizeof(struct ni_info))) {
> + if (copy_from_user(&ni, uarg, sizeof(ni))) {
> return -EFAULT;
> }
>
> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
> index fe38529..894b7df 100644
> --- a/drivers/staging/dgnc/dgnc_tty.c
> +++ b/drivers/staging/dgnc/dgnc_tty.c
> @@ -200,8 +200,8 @@ int dgnc_tty_register(struct dgnc_board *brd)
>
> DPR_INIT(("tty_register start\n"));
>
> - memset(&brd->SerialDriver, 0, sizeof(struct tty_driver));
> - memset(&brd->PrintDriver, 0, sizeof(struct tty_driver));
> + memset(&brd->SerialDriver, 0, sizeof(brd->SerialDriver));
> + memset(&brd->PrintDriver, 0, sizeof(brd->PrintDriver));
>
> brd->SerialDriver.magic = TTY_DRIVER_MAGIC;
>
> @@ -222,12 +222,12 @@ int dgnc_tty_register(struct dgnc_board *brd)
> * The kernel wants space to store pointers to
> * tty_struct's and termios's.
> */
> - brd->SerialDriver.ttys = kzalloc(brd->maxports * sizeof(struct
> tty_struct *), GFP_KERNEL);
> + brd->SerialDriver.ttys = kzalloc(brd->maxports *
> sizeof(brd->SerialDriver.ttys), GFP_KERNEL);
ttys is a pointer to a pointer. What you have works but it would be
better to say. sizeof(*brd->SerialDriver.ttys). For this one kcalloc()
is actually better than kzalloc(). It's cleaner and it has overflow
protection built in.
> if (!brd->SerialDriver.ttys)
> return -ENOMEM;
>
> kref_init(&brd->SerialDriver.kref);
> - brd->SerialDriver.termios = kzalloc(brd->maxports * sizeof(struct
> ktermios *), GFP_KERNEL);
> + brd->SerialDriver.termios = kzalloc(brd->maxports *
> sizeof(brd->SerialDriver.termios), GFP_KERNEL);
Same.
> if (!brd->SerialDriver.termios)
> return -ENOMEM;
>
> @@ -271,11 +271,11 @@ int dgnc_tty_register(struct dgnc_board *brd)
> * tty_struct's and termios's. Must be seperate from
> * the Serial Driver so we don't get confused
> */
> - brd->PrintDriver.ttys = kzalloc(brd->maxports * sizeof(struct
> tty_struct *), GFP_KERNEL);
> + brd->PrintDriver.ttys = kzalloc(brd->maxports *
> sizeof(brd->PrintDriver.ttys), GFP_KERNEL);
Same.
> if (!brd->PrintDriver.ttys)
> return -ENOMEM;
> kref_init(&brd->PrintDriver.kref);
> - brd->PrintDriver.termios = kzalloc(brd->maxports * sizeof(struct
> ktermios *), GFP_KERNEL);
> + brd->PrintDriver.termios = kzalloc(brd->maxports *
> sizeof(brd->PrintDriver.termios), GFP_KERNEL);
Same.
> if (!brd->PrintDriver.termios)
> return -ENOMEM;
>
> @@ -341,7 +341,7 @@ int dgnc_tty_init(struct dgnc_board *brd)
> * Okay to malloc with GFP_KERNEL, we are not at
> * interrupt context, and there are no locks held.
> */
> - brd->channels[i] = kzalloc(sizeof(struct channel_t),
> GFP_KERNEL);
> + brd->channels[i] = kzalloc(sizeof(brd->channels[i]),
> GFP_KERNEL);
This one is buggy. It should be:
brd->channels[i] = kzalloc(sizeof(*brd->channels[i]),
GFP_KERNEL);
But really that's awkward so the original is probably fine.
regards,
dan carpenter
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
