Re: [PATCH] cleanup in drivers/mtd/ftl.c (245-ac16)
On Fri, Jun 22, 2001 at 05:21:06PM -0300, Arnaldo Carvalho de Melo wrote: > Hi Rasmus, > > I've fixed this ones and its already in 2.4.6-pre5, please take a > look and see if something is missing. These patches are very close so I'll of course retract mine[1]. The only thing I'll recommend is the printk I have in the error path. [1] Sorry about the unnecessary mailing. I am not accustomed to janitor-like patches being in Linus' kernel before Alan's :) (Arnaldo already offered an explanation for why this one happened to be.) -- Regards, Rasmus([EMAIL PROTECTED]) Smoking kills. If you're killed, you've lost a very important part of your life. -Brooke Shields, during an interview to become spokesperson for a federal anti-smoking campaign. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cleanup in drivers/mtd/ftl.c (245-ac16)
Hi Rasmus, I've fixed this ones and its already in 2.4.6-pre5, please take a look and see if something is missing. - Arnaldo Em Fri, Jun 22, 2001 at 10:29:31PM +0200, Rasmus Andersen escreveu: > Hi. > > The patch below adds one instance of vmalloc return code checking > and a number of error path resource release cleanups in build_maps. > It is against 245-ac16. > > (The vmalloc non-check was reported by the Stanford team a > while back.) > > > --- linux-245-ac16-clean/drivers/mtd/ftl.cSun May 27 22:15:23 2001 > +++ linux-245-ac16/drivers/mtd/ftl.c Fri Jun 22 22:24:09 2001 > @@ -313,6 +313,7 @@ > ssize_t retval; > loff_t offset; > > +ret = -1; > /* Set up erase unit maps */ > part->DataUnits = le16_to_cpu(part->header.NumEraseUnits) - > part->header.NumTransferUnits; > @@ -324,7 +325,8 @@ > part->XferInfo = > kmalloc(part->header.NumTransferUnits * sizeof(struct xfer_info_t), > GFP_KERNEL); > -if (!part->XferInfo) return -1; > +if (!part->XferInfo) > + goto err_free_EUInfo; > > xvalid = xtrans = 0; > for (i = 0; i < le16_to_cpu(part->header.NumEraseUnits); i++) { > @@ -334,8 +336,9 @@ > (unsigned char *)&header); > > if (ret) > - return ret; > + goto err_free_XferInfo; > > + ret = -1; > /* Is this a transfer partition? */ > hdr_ok = (strcmp(header.DataOrgTuple+3, "FTL100") == 0); > if (hdr_ok && (le16_to_cpu(header.LogicalEUN) < part->DataUnits) && > @@ -348,7 +351,7 @@ > if (xtrans == part->header.NumTransferUnits) { > printk(KERN_NOTICE "ftl_cs: format error: too many " > "transfer units!\n"); > - return -1; > + goto err_free_XferInfo; > } > if (hdr_ok && (le16_to_cpu(header.LogicalEUN) == 0x)) { > part->XferInfo[xtrans].state = XFER_PREPARED; > @@ -369,18 +372,21 @@ > (xvalid+xtrans != le16_to_cpu(header.NumEraseUnits))) { > printk(KERN_NOTICE "ftl_cs: format error: erase units " > "don't add up!\n"); > - return -1; > + goto err_free_XferInfo; > } > > /* Set up virtual page map */ > blocks = le32_to_cpu(header.FormattedSize) >> header.BlockSize; > part->VirtualBlockMap = vmalloc(blocks * sizeof(u_int32_t)); > +if (!part->VirtualBlockMap) > + goto err_free_XferInfo; > memset(part->VirtualBlockMap, 0xff, blocks * sizeof(u_int32_t)); > part->BlocksPerUnit = (1 << header.EraseUnitSize) >> header.BlockSize; > > part->bam_cache = kmalloc(part->BlocksPerUnit * sizeof(u_int32_t), > GFP_KERNEL); > -if (!part->bam_cache) return -1; > +if (!part->bam_cache) > + goto err_free_VirtualBlockMap; > > part->bam_index = 0x; > part->FreeTotal = 0; > @@ -395,7 +401,7 @@ > (unsigned char *)part->bam_cache); > > if (ret) > - return ret; > + goto err_free_bam_cache; > > for (j = 0; j < part->BlocksPerUnit; j++) { > if (BLOCK_FREE(le32_to_cpu(part->bam_cache[j]))) { > @@ -411,7 +417,17 @@ > } > > return 0; > - > + > +err_free_bam_cache: > +kfree(part->bam_cache); > +err_free_VirtualBlockMap: > +vfree(part->VirtualBlockMap); > +err_free_XferInfo: > +kfree(part->XferInfo); > +err_free_EUInfo: > +kfree(part->EUNInfo); > +printk(KERN_ERR "ftl_cs: Out of memory."); > +return ret; > } /* build_maps */ > > /*== > > -- > Regards, > Rasmus([EMAIL PROTECTED]) > > Open Source. Closed Minds. We are Slashdot. > --Anonymous Coward > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- - Arnaldo - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] cleanup in drivers/mtd/ftl.c (245-ac16)
Hi. The patch below adds one instance of vmalloc return code checking and a number of error path resource release cleanups in build_maps. It is against 245-ac16. (The vmalloc non-check was reported by the Stanford team a while back.) --- linux-245-ac16-clean/drivers/mtd/ftl.c Sun May 27 22:15:23 2001 +++ linux-245-ac16/drivers/mtd/ftl.cFri Jun 22 22:24:09 2001 @@ -313,6 +313,7 @@ ssize_t retval; loff_t offset; +ret = -1; /* Set up erase unit maps */ part->DataUnits = le16_to_cpu(part->header.NumEraseUnits) - part->header.NumTransferUnits; @@ -324,7 +325,8 @@ part->XferInfo = kmalloc(part->header.NumTransferUnits * sizeof(struct xfer_info_t), GFP_KERNEL); -if (!part->XferInfo) return -1; +if (!part->XferInfo) + goto err_free_EUInfo; xvalid = xtrans = 0; for (i = 0; i < le16_to_cpu(part->header.NumEraseUnits); i++) { @@ -334,8 +336,9 @@ (unsigned char *)&header); if (ret) - return ret; + goto err_free_XferInfo; + ret = -1; /* Is this a transfer partition? */ hdr_ok = (strcmp(header.DataOrgTuple+3, "FTL100") == 0); if (hdr_ok && (le16_to_cpu(header.LogicalEUN) < part->DataUnits) && @@ -348,7 +351,7 @@ if (xtrans == part->header.NumTransferUnits) { printk(KERN_NOTICE "ftl_cs: format error: too many " "transfer units!\n"); - return -1; + goto err_free_XferInfo; } if (hdr_ok && (le16_to_cpu(header.LogicalEUN) == 0x)) { part->XferInfo[xtrans].state = XFER_PREPARED; @@ -369,18 +372,21 @@ (xvalid+xtrans != le16_to_cpu(header.NumEraseUnits))) { printk(KERN_NOTICE "ftl_cs: format error: erase units " "don't add up!\n"); - return -1; + goto err_free_XferInfo; } /* Set up virtual page map */ blocks = le32_to_cpu(header.FormattedSize) >> header.BlockSize; part->VirtualBlockMap = vmalloc(blocks * sizeof(u_int32_t)); +if (!part->VirtualBlockMap) + goto err_free_XferInfo; memset(part->VirtualBlockMap, 0xff, blocks * sizeof(u_int32_t)); part->BlocksPerUnit = (1 << header.EraseUnitSize) >> header.BlockSize; part->bam_cache = kmalloc(part->BlocksPerUnit * sizeof(u_int32_t), GFP_KERNEL); -if (!part->bam_cache) return -1; +if (!part->bam_cache) + goto err_free_VirtualBlockMap; part->bam_index = 0x; part->FreeTotal = 0; @@ -395,7 +401,7 @@ (unsigned char *)part->bam_cache); if (ret) - return ret; + goto err_free_bam_cache; for (j = 0; j < part->BlocksPerUnit; j++) { if (BLOCK_FREE(le32_to_cpu(part->bam_cache[j]))) { @@ -411,7 +417,17 @@ } return 0; - + +err_free_bam_cache: +kfree(part->bam_cache); +err_free_VirtualBlockMap: +vfree(part->VirtualBlockMap); +err_free_XferInfo: +kfree(part->XferInfo); +err_free_EUInfo: +kfree(part->EUNInfo); +printk(KERN_ERR "ftl_cs: Out of memory."); +return ret; } /* build_maps */ /*== -- Regards, Rasmus([EMAIL PROTECTED]) Open Source. Closed Minds. We are Slashdot. --Anonymous Coward - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/