Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912 
was reviewed by Gedare Bloom

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139445

 >      return RTEMS_NO_MEMORY;
 >  
 > +  bfd = fd;

what's the purpose of this?

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139446

 > +static void rtems_fdisk_cleanup_helper (rtems_flashdisk *fd)
 > +{
 > +  if(!fd)

please see https://docs.rtems.org/docs/main/eng/coding-conventions.html
for coding style. we should have a format tool available soon, but here you 
should also note preference to explicitly check `if ( fd == NULL )`.

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139447

 > +    return;
 > +
 > +  uint32_t device;

declare variables at the start of the code block.

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139448

 > +    for(device = 0; device< fd->device_count; device++)
 > +    {
 > +      free(fd->devices[device].segments);

do you need to check if they are non-NULL?

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139449

 > +    }
 > +  }
 > +  free(fd->devices);

this should be inside the `if` part

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139450

 > +  fd->devices = NULL;
 > +
 > +  free(fd->blocks);

should you check if they are non-NULL? Are they initialized to NULL?

--
  
Gedare Bloom started a new discussion on cpukit/libblock/src/flashdisk.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912#note_139451

 > +    if(count==0)
 > +      break;
 > +    --count;

this should be a `for` loop.


-- 
View it on GitLab: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/912
You're receiving this email because of your account on gitlab.rtems.org.


_______________________________________________
bugs mailing list
[email protected]
http://lists.rtems.org/mailman/listinfo/bugs

Reply via email to