On Fri, Jun 4, 2021 at 1:17 PM Joel Sherrill <j...@rtems.org> wrote: > > On the surface, this looks OK to me. But I remember looking at this one > and wondering if there was any cleanup required because of the various > subroutine calls as you work down through this method. > > Did you look into each of the subroutines called and make sure they > didn't do further allocations? > We hadn't. Now, we took a closer look at this also, and see some more problems that need to be addressed. The fd variable is a loop iterator, so we can't just free() it. Should this function cleanup everything that was attempted and not to return with a partial initialization in case of a failure? Or, should it allow the partial initialization of some of the fd's, and just cleanup the last one that fails (due to out of memory)?
> --joel > > On Fri, Jun 4, 2021 at 1:27 PM Harrison Edward Gerber <gerberh...@gmail.com> > wrote: >> >> See also CID 1439298 >> >> Closes #3570 >> --- >> cpukit/libblock/src/flashdisk.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/cpukit/libblock/src/flashdisk.c >> b/cpukit/libblock/src/flashdisk.c >> index 91f99e0d52..4de6ecd807 100644 >> --- a/cpukit/libblock/src/flashdisk.c >> +++ b/cpukit/libblock/src/flashdisk.c >> @@ -2486,17 +2486,29 @@ rtems_fdisk_initialize (rtems_device_major_number >> major, >> */ >> fd->copy_buffer = malloc (c->block_size); >> if (!fd->copy_buffer) >> + { >> + free(fd); >> return RTEMS_NO_MEMORY; >> + } >> >> fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl)); >> 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) >> + { >> + free(fd->blocks); >> + free(fd->copy_buffer); >> + free(fd); >> return RTEMS_NO_MEMORY; >> + } >> >> rtems_mutex_init (&fd->lock, "Flash Disk"); >> >> @@ -2508,6 +2520,7 @@ rtems_fdisk_initialize (rtems_device_major_number >> major, >> free (fd->copy_buffer); >> free (fd->blocks); >> free (fd->devices); >> + free(fd); >> rtems_fdisk_error ("disk create phy failed"); >> return sc; >> } >> @@ -2529,6 +2542,7 @@ rtems_fdisk_initialize (rtems_device_major_number >> major, >> free (fd->copy_buffer); >> free (fd->blocks); >> free (fd->devices); >> + free(fd); >> return RTEMS_NO_MEMORY; >> } >> >> @@ -2564,6 +2578,7 @@ rtems_fdisk_initialize (rtems_device_major_number >> major, >> free (fd->copy_buffer); >> free (fd->blocks); >> free (fd->devices); >> + free(fd); >> rtems_fdisk_error ("recovery of disk failed: %s (%d)", >> strerror (ret), ret); >> return ret; >> @@ -2577,6 +2592,7 @@ rtems_fdisk_initialize (rtems_device_major_number >> major, >> free (fd->copy_buffer); >> free (fd->blocks); >> free (fd->devices); >> + free(fd); >> rtems_fdisk_error ("compacting of disk failed: %s (%d)", >> strerror (ret), ret); >> return ret; >> -- >> 2.25.1 >> >> _______________________________________________ >> devel mailing list >> devel@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel