On Monday, December 28, 2009 9:51 AM, Julia Lawall wrote: > On Mon, 28 Dec 2009, H Hartley Sweeten wrote: > >> On Sunday, December 27, 2009 3:08 PM, Julia Lawall wrote: >>>>> dbgfs_chan = kmalloc(sizeof(*dbgfs_state) * num_dma_channels, >>>>> GFP_KERNEL); >>>>> - if (!dbgfs_state) >>>>> + if (!dbgfs_chan) >>>>> goto err_alloc; >>>> Shouldn't the malloc line read: >>>> >>>> ... = kmalloc(sizeof(*dbgfs_chan) * ...) >>>> ^^^^^^^^^^ >>>> >>>> ? >>> >>> Good point, thanks. I will send a revised patch. >> >> Wouldn't this be clearer? >> >> ... = kmalloc(sizeof(struct dentry) * ...) > > Documentation/CodingStyle thinks otherwise: > > "The preferred form for passing a size of a struct is the following: > > p = kmalloc(sizeof(*p), ...); > > The alternative form where struct name is spelled out hurts readability > and introduces an opportunity for a bug when the pointer variable type is > changed but the corresponding sizeof that is passed to a memory allocator > is not." > > And actually, in this case, sizeof(struct dentry) would be wrong, because > the type of dbgfs_chan is struct dentry **, not struct dentry *. What is > wanted is an array of pointers. Ah, missed that. Thanks for pointing it out. Regards, Hartley -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html