On 15/3/21 2:21 pm, Joel Sherrill wrote: > On Sun, Mar 14, 2021, 9:27 PM Chris Johns <chr...@rtems.org > <mailto:chr...@rtems.org>> wrote: > On 13/3/21 2:18 am, Ryan Long wrote: > > CID 1439298: Resource leak in rtems_fdisk_initialize(). > > > > Closes #4299 > > --- > > cpukit/libblock/src/flashdisk.c | 42 > ++++++++++++++++++++++++++++++----------- > > 1 file changed, 31 insertions(+), 11 deletions(-) > > > > diff --git a/cpukit/libblock/src/flashdisk.c > b/cpukit/libblock/src/flashdisk.c > > index 91f99e0..c4bac82 100644 > > --- a/cpukit/libblock/src/flashdisk.c > > +++ b/cpukit/libblock/src/flashdisk.c > > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize (rtems_device_major_number > major, > > { > > char name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a"; > > uint32_t device; > > + uint32_t device_to_free; > > uint32_t blocks = 0; > > int ret; > > > > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize > (rtems_device_major_number > major, > > * One copy buffer of a page size. > > */ > > fd->copy_buffer = malloc (c->block_size); > > - if (!fd->copy_buffer) > > + if (!fd->copy_buffer) { > > + free(fd); > > return RTEMS_NO_MEMORY; > > + } > > > > fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl)); > > - if (!fd->blocks) > > + if (!fd->blocks) { > > + free(fd->copy_buffer); > > + free(fd); > > return RTEMS_NO_MEMORY; > > + } > > > > fd->block_count = blocks; > > > > fd->devices = calloc (c->device_count, sizeof > (rtems_fdisk_device_ctl)); > > - if (!fd->devices) > > + if (!fd->devices) { > > + free (fd->blocks); > > + free (fd->copy_buffer); > > + free (fd); > > return RTEMS_NO_MEMORY; > > + } > > > > rtems_mutex_init (&fd->lock, "Flash Disk"); > > > > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize (rtems_device_major_number > major, > > if (sc != RTEMS_SUCCESSFUL) > > { > > rtems_mutex_destroy (&fd->lock); > > - free (fd->copy_buffer); > > - free (fd->blocks); > > free (fd->devices); > > + free (fd->blocks); > > + free (fd->copy_buffer); > > Why the order change? > > Does the change make it exactly the opposite order of creation or do you see > it > not being in inverse order?
If there is no reason to change the order the blocks are freed then I suggest not changing the order. It avoids adding noise to the change. > This was a hard one. It was missing a LOT of cleanup. > > > > + free (fd); > > What happens to the created blkdev the fd is passed into? Does that need > to be > destroyed before this is released? I do not know. I have not looked. > I didn't recognise that as an allocation. What's the destroy call for that? If the block dev holds the pointer and you have freed it bad things will happen. I have a funny feeling there was no block dev destroy when the code was written and why there are no free's for this allocation. Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
