Re: [PATCH] some kmalloc/memset ->kzalloc (tree wide)

[Tilman Schmidt <tilman@imap.cc>]

Am 06.07.2007 18:51 schrieb Yoann Padioleau:
> @@
> expression E1,E2,E3;
> @@
> 
> - kzalloc(E1 * E2,E3)
> + kcalloc(E1,E2,E3)

This misses the semantic distinction between the first and second
arguments of kcalloc(). The first argument is supposed to be the
number of elements to allocate and the second their size. As a
consequence, the following hunks in your pathc are wrong:

> diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
> index 9d63d7f..b84955f 100644
> --- a/arch/arm/mach-iop13xx/pci.c
> +++ b/arch/arm/mach-iop13xx/pci.c
> @@ -1002,11 +1002,10 @@ int iop13xx_pci_setup(int nr, struct pci
>  	if (nr > 1)
>  		return 0;
>  
> -	res = kmalloc(sizeof(struct resource) * 2, GFP_KERNEL);
> +	res = kcalloc(sizeof(struct resource), 2, GFP_KERNEL);
>  	if (!res)
>  		panic("PCI: unable to alloc resources");
>  
> -	memset(res, 0, sizeof(struct resource) * 2);
>  
>  	/* 'nr' assumptions:
>  	 * ATUX is always 0

> diff --git a/drivers/char/drm/via_dmablit.c b/drivers/char/drm/via_dmablit.c
> index 2881a06..f246109 100644
> --- a/drivers/char/drm/via_dmablit.c
> +++ b/drivers/char/drm/via_dmablit.c
> @@ -273,10 +273,9 @@ via_alloc_desc_pages(drm_via_sg_info_t *
>  	vsg->num_desc_pages = (vsg->num_desc + vsg->descriptors_per_page - 1) / 
>  		vsg->descriptors_per_page;
>  
> -	if (NULL ==  (vsg->desc_pages = kmalloc(sizeof(void *) * vsg->num_desc_pages, GFP_KERNEL))) 
> +	if (NULL ==  (vsg->desc_pages = kcalloc(sizeof(void *), vsg->num_desc_pages, GFP_KERNEL))) 
>  		return DRM_ERR(ENOMEM);
>  	
> -	memset(vsg->desc_pages, 0, sizeof(void *) * vsg->num_desc_pages);
>  	vsg->state = dr_via_desc_pages_alloc;
>  	for (i=0; i<vsg->num_desc_pages; ++i) {
>  		if (NULL == (vsg->desc_pages[i] = 

> diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
> index 42ba1c0..d383455 100644
> --- a/drivers/net/forcedeth.c
> +++ b/drivers/net/forcedeth.c
> @@ -4995,12 +4995,10 @@ static int __devinit nv_probe(struct pci
>  			goto out_unmap;
>  		np->tx_ring.ex = &np->rx_ring.ex[np->rx_ring_size];
>  	}
> -	np->rx_skb = kmalloc(sizeof(struct nv_skb_map) * np->rx_ring_size, GFP_KERNEL);
> -	np->tx_skb = kmalloc(sizeof(struct nv_skb_map) * np->tx_ring_size, GFP_KERNEL);
> +	np->rx_skb = kcalloc(sizeof(struct nv_skb_map), np->rx_ring_size, GFP_KERNEL);
> +	np->tx_skb = kcalloc(sizeof(struct nv_skb_map), np->tx_ring_size, GFP_KERNEL);
>  	if (!np->rx_skb || !np->tx_skb)
>  		goto out_freering;
> -	memset(np->rx_skb, 0, sizeof(struct nv_skb_map) * np->rx_ring_size);
> -	memset(np->tx_skb, 0, sizeof(struct nv_skb_map) * np->tx_ring_size);
>  
>  	dev->open = nv_open;
>  	dev->stop = nv_close;

> diff --git a/drivers/net/wan/cosa.c b/drivers/net/wan/cosa.c
> index 9ef49ce..89a06b3 100644
> --- a/drivers/net/wan/cosa.c
> +++ b/drivers/net/wan/cosa.c
> @@ -572,13 +572,11 @@ #endif
>  	sprintf(cosa->name, "cosa%d", cosa->num);
>  
>  	/* Initialize the per-channel data */
> -	cosa->chan = kmalloc(sizeof(struct channel_data)*cosa->nchannels,
> -			     GFP_KERNEL);
> +	cosa->chan = kcalloc(sizeof(struct channel_data), cosa->nchannels, GFP_KERNEL);
>  	if (!cosa->chan) {
>  	        err = -ENOMEM;
>  		goto err_out3;
>  	}
> -	memset(cosa->chan, 0, sizeof(struct channel_data)*cosa->nchannels);
>  	for (i=0; i<cosa->nchannels; i++) {
>  		cosa->chan[i].cosa = cosa;
>  		cosa->chan[i].num = i;

> diff --git a/drivers/net/wan/cycx_main.c b/drivers/net/wan/cycx_main.c
> index 6e5f1c8..8b6f22c 100644
> --- a/drivers/net/wan/cycx_main.c
> +++ b/drivers/net/wan/cycx_main.c
> @@ -113,12 +113,10 @@ static int __init cycx_init(void)
>  	/* Verify number of cards and allocate adapter data space */
>  	cycx_ncards = min_t(int, cycx_ncards, CYCX_MAX_CARDS);
>  	cycx_ncards = max_t(int, cycx_ncards, 1);
> -	cycx_card_array = kmalloc(sizeof(struct cycx_device) * cycx_ncards,
> -				  GFP_KERNEL);
> +	cycx_card_array = kcalloc(sizeof(struct cycx_device), cycx_ncards, GFP_KERNEL);
>  	if (!cycx_card_array)
>  		goto out;
>  
> -	memset(cycx_card_array, 0, sizeof(struct cycx_device) * cycx_ncards);
>  
>  	/* Register adapters with WAN router */
>  	for (cnt = 0; cnt < cycx_ncards; ++cnt) {

> diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
> index 1c9edd9..5bceb2c 100644
> --- a/drivers/net/wan/x25_asy.c
> +++ b/drivers/net/wan/x25_asy.c
> @@ -786,14 +786,12 @@ static int __init init_x25_asy(void)
>  	printk(KERN_INFO "X.25 async: version 0.00 ALPHA "
>  			"(dynamic channels, max=%d).\n", x25_asy_maxdev );
>  
> -	x25_asy_devs = kmalloc(sizeof(struct net_device *)*x25_asy_maxdev, 
> -			       GFP_KERNEL);
> +	x25_asy_devs = kcalloc(sizeof(struct net_device*), x25_asy_maxdev, GFP_KERNEL);
>  	if (!x25_asy_devs) {
>  		printk(KERN_WARNING "X25 async: Can't allocate x25_asy_ctrls[] "
>  				"array! Uaargh! (-> No X.25 available)\n");
>  		return -ENOMEM;
>  	}
> -	memset(x25_asy_devs, 0, sizeof(struct net_device *)*x25_asy_maxdev); 
>  
>  	return tty_register_ldisc(N_X25, &x25_ldisc);
>  }

> diff --git a/drivers/sbus/char/vfc_dev.c b/drivers/sbus/char/vfc_dev.c
> index 6afc7e5..4b5918b 100644
> --- a/drivers/sbus/char/vfc_dev.c
> +++ b/drivers/sbus/char/vfc_dev.c
> @@ -656,12 +656,9 @@ static int vfc_probe(void)
>  	if (!cards)
>  		return -ENODEV;
>  
> -	vfc_dev_lst = kmalloc(sizeof(struct vfc_dev *) *
> -						 (cards+1),
> -						 GFP_KERNEL);
> +	vfc_dev_lst = kcalloc(sizeof(struct vfc_dev*), (cards+1), GFP_KERNEL);
>  	if (vfc_dev_lst == NULL)
>  		return -ENOMEM;
> -	memset(vfc_dev_lst, 0, sizeof(struct vfc_dev *) * (cards + 1));
>  	vfc_dev_lst[cards] = NULL;
>  
>  	ret = register_chrdev(VFC_MAJOR, vfcstr, &vfc_fops);

> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index eb766c3..09b0e5c 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -1160,13 +1160,12 @@ static int twa_initialize_device_extensi
>  	}
>  
>  	/* Allocate event info space */
> -	tw_dev->event_queue[0] = kmalloc(sizeof(TW_Event) * TW_Q_LENGTH, GFP_KERNEL);
> +	tw_dev->event_queue[0] = kcalloc(sizeof(TW_Event), TW_Q_LENGTH, GFP_KERNEL);
>  	if (!tw_dev->event_queue[0]) {
>  		TW_PRINTK(tw_dev->host, TW_DRIVER, 0x18, "Event info memory allocation failed");
>  		goto out;
>  	}
>  
> -	memset(tw_dev->event_queue[0], 0, sizeof(TW_Event) * TW_Q_LENGTH);
>  
>  	for (i = 0; i < TW_Q_LENGTH; i++) {
>  		tw_dev->event_queue[i] = (TW_Event *)((unsigned char *)tw_dev->event_queue[0] + (i * sizeof(TW_Event)));

> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 8dcfe4e..81e27ed 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -312,11 +312,9 @@ int aac_get_containers(struct aac_dev *d
>  
>  	if (maximum_num_containers < MAXIMUM_NUM_CONTAINERS)
>  		maximum_num_containers = MAXIMUM_NUM_CONTAINERS;
> -	fsa_dev_ptr =  kmalloc(sizeof(*fsa_dev_ptr) * maximum_num_containers,
> -			GFP_KERNEL);
> +	fsa_dev_ptr =  kcalloc(sizeof(*fsa_dev_ptr), maximum_num_containers, GFP_KERNEL);
>  	if (!fsa_dev_ptr)
>  		return -ENOMEM;
> -	memset(fsa_dev_ptr, 0, sizeof(*fsa_dev_ptr) * maximum_num_containers);
>  
>  	dev->fsa_dev = fsa_dev_ptr;
>  	dev->maximum_num_containers = maximum_num_containers;

> diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
> index 04d0b69..8cfa5cd 100644
> --- a/drivers/scsi/megaraid/megaraid_mbox.c
> +++ b/drivers/scsi/megaraid/megaraid_mbox.c
> @@ -1050,8 +1048,7 @@ megaraid_alloc_cmd_packets(adapter_t *ad
>  	 * since the calling routine does not yet know the number of available
>  	 * commands.
>  	 */
> -	adapter->kscb_list = kmalloc(sizeof(scb_t) * MBOX_MAX_SCSI_CMDS,
> -			GFP_KERNEL);
> +	adapter->kscb_list = kcalloc(sizeof(scb_t), MBOX_MAX_SCSI_CMDS, GFP_KERNEL);
>  
>  	if (adapter->kscb_list == NULL) {
>  		con_log(CL_ANN, (KERN_WARNING
> @@ -1059,7 +1056,6 @@ megaraid_alloc_cmd_packets(adapter_t *ad
>  			__LINE__));
>  		goto out_free_ibuf;
>  	}
> -	memset(adapter->kscb_list, 0, sizeof(scb_t) * MBOX_MAX_SCSI_CMDS);
>  
>  	// memory allocation for our command packets
>  	if (megaraid_mbox_setup_dma_pools(adapter) != 0) {
> @@ -3572,8 +3568,7 @@ megaraid_cmm_register(adapter_t *adapter
>  	int		i;
>  
>  	// Allocate memory for the base list of scb for management module.
> -	adapter->uscb_list = kmalloc(sizeof(scb_t) * MBOX_MAX_USER_CMDS,
> -			GFP_KERNEL);
> +	adapter->uscb_list = kcalloc(sizeof(scb_t), MBOX_MAX_USER_CMDS, GFP_KERNEL);
>  
>  	if (adapter->uscb_list == NULL) {
>  		con_log(CL_ANN, (KERN_WARNING
> @@ -3581,7 +3576,6 @@ megaraid_cmm_register(adapter_t *adapter
>  			__LINE__));
>  		return -1;
>  	}
> -	memset(adapter->uscb_list, 0, sizeof(scb_t) * MBOX_MAX_USER_CMDS);
>  
>  
>  	// Initialize the synchronization parameters for resources for

> diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
> index e2cf12e..d2bec2b 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.c
> +++ b/drivers/scsi/megaraid/megaraid_sas.c
> @@ -1714,15 +1714,13 @@ static int megasas_alloc_cmds(struct meg
>  	 * Allocate the dynamic array first and then allocate individual
>  	 * commands.
>  	 */
> -	instance->cmd_list = kmalloc(sizeof(struct megasas_cmd *) * max_cmd,
> -				     GFP_KERNEL);
> +	instance->cmd_list = kcalloc(sizeof(struct megasas_cmd*), max_cmd, GFP_KERNEL);
>  
>  	if (!instance->cmd_list) {
>  		printk(KERN_DEBUG "megasas: out of memory\n");
>  		return -ENOMEM;
>  	}
>  
> -	memset(instance->cmd_list, 0, sizeof(struct megasas_cmd *) * max_cmd);
>  
>  	for (i = 0; i < max_cmd; i++) {
>  		instance->cmd_list[i] = kmalloc(sizeof(struct megasas_cmd),

> diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
> index dbf4ec3..311e323 100644
> --- a/drivers/video/au1200fb.c
> +++ b/drivers/video/au1200fb.c
> @@ -1589,11 +1589,10 @@ static int au1200fb_init_fbinfo(struct a
>  		return -EFAULT;
>  	}
>  
> -	fbi->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
> +	fbi->pseudo_palette = kcalloc(sizeof(u32), 16, GFP_KERNEL);
>  	if (!fbi->pseudo_palette) {
>  		return -ENOMEM;
>  	}
> -	memset(fbi->pseudo_palette, 0, sizeof(u32) * 16);
>  
>  	if (fb_alloc_cmap(&fbi->cmap, AU1200_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
>  		print_err("Fail to allocate colormap (%d entries)",

> diff --git a/drivers/video/savage/savagefb_driver.c b/drivers/video/savage/savagefb_driver.c
> index 3d7507a..b855f4a 100644
> --- a/drivers/video/savage/savagefb_driver.c
> +++ b/drivers/video/savage/savagefb_driver.c
> @@ -2174,11 +2174,10 @@ static int __devinit savage_init_fb_info
>  
>  #if defined(CONFIG_FB_SAVAGE_ACCEL)
>  	/* FIFO size + padding for commands */
> -	info->pixmap.addr = kmalloc(8*1024, GFP_KERNEL);
> +	info->pixmap.addr = kcalloc(8, 1024, GFP_KERNEL);
>  
>  	err = -ENOMEM;
>  	if (info->pixmap.addr) {
> -		memset(info->pixmap.addr, 0, 8*1024);
>  		info->pixmap.size = 8*1024;
>  		info->pixmap.scan_align = 4;
>  		info->pixmap.buf_align = 4;

HTH

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Please excuse my bad English/German/French/Greek/Cantonese/Klingon/...

Attachment: signature.asc
Description: OpenPGP digital signature