| Message ID | 1462242428-583-1-git-send-email-andrew.donnellan@au1.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, 3 May 2016 12:27:08 +1000 Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote: > Some error paths in flash_setup_buffer() don't free the flash_info struct > allocated in that function before they return. Change them to goto the > cleanup code at the end. Separate the cleanup code into separate labels for > calling arch_flash_close() and just freeing memory. Looks good, I know talloc can be rather magic but surely it can't work that out... While you're at it though, missed one... > > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > > --- > > Found by Coverity Scan. Build-tested only. sammj, please bikeshed :) > --- > lib/flash/flash.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/lib/flash/flash.c b/lib/flash/flash.c > index b6188a0..107ba9e 100644 > --- a/lib/flash/flash.c > +++ b/lib/flash/flash.c > @@ -82,26 +82,26 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition) > rc = arch_flash_init(&info->bl, NULL, true); > if (rc) { > pb_log("Failed to init mtd device\n"); > - return NULL; > + goto out; > } > > rc = blocklevel_get_info(info->bl, &info->path, &info->size, > &info->erase_granule); > if (rc) { > pb_log("Failed to retrieve blocklevel info\n"); > - return NULL; > + goto out_close; > } > > rc = ffs_init(0, info->size, info->bl, &info->ffs, 1); > if (rc) { > pb_log("%s: Failed to init ffs\n", __func__); > - goto out; > + goto out_close; > } > > rc = partition_info(info, partition); > if (rc) { > pb_log("Failed to retrieve partition info\n"); ffs_close(info->ffs); > - goto out; > + goto out_close; > } > > /* Check if there is a second flash side. If there is not, or > @@ -139,8 +139,9 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition) > } > > return info; > -out: > +out_close: > arch_flash_close(info->bl, NULL); > +out: > talloc_free(info); > return NULL; > }
On 03/05/16 18:07, Cyril Bur wrote: > Looks good, I know talloc can be rather magic but surely it can't work that > out... > > While you're at it though, missed one... > > ffs_close(info->ffs); Thanks Cyril - will submit V2
diff --git a/lib/flash/flash.c b/lib/flash/flash.c index b6188a0..107ba9e 100644 --- a/lib/flash/flash.c +++ b/lib/flash/flash.c @@ -82,26 +82,26 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition) rc = arch_flash_init(&info->bl, NULL, true); if (rc) { pb_log("Failed to init mtd device\n"); - return NULL; + goto out; } rc = blocklevel_get_info(info->bl, &info->path, &info->size, &info->erase_granule); if (rc) { pb_log("Failed to retrieve blocklevel info\n"); - return NULL; + goto out_close; } rc = ffs_init(0, info->size, info->bl, &info->ffs, 1); if (rc) { pb_log("%s: Failed to init ffs\n", __func__); - goto out; + goto out_close; } rc = partition_info(info, partition); if (rc) { pb_log("Failed to retrieve partition info\n"); - goto out; + goto out_close; } /* Check if there is a second flash side. If there is not, or @@ -139,8 +139,9 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition) } return info; -out: +out_close: arch_flash_close(info->bl, NULL); +out: talloc_free(info); return NULL; }
Some error paths in flash_setup_buffer() don't free the flash_info struct allocated in that function before they return. Change them to goto the cleanup code at the end. Separate the cleanup code into separate labels for calling arch_flash_close() and just freeing memory. Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> --- Found by Coverity Scan. Build-tested only. sammj, please bikeshed :) --- lib/flash/flash.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)