RTEMS | IMFS User allocator Plugin Code (!534)

[Gedare Bloom (@gedare)]

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

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/confdefs/libio.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125629

 > +  #include<rtems/imfs.h>
 > +    extern IMFS_memfile_ops imfs_memfile_ops;
 > +  #endif

why is this after the c++ guard?

also, what is this `#else` on? the context is lost here.

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/confdefs/libio.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125630

 > +#endif
 > +
 > +

only 1 blank line

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/confdefs/libio.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125631

 > +  IMFS_memfile_ops imfs_memfile_ops = CONFIGURE_IMFS_MEMFILE_OPS;
 > +#else
 > +  IMFS_memfile_ops imfs_memfile_ops = {.allocator = NULL, .deallocator = NULL, .free_space = NULL};

declaring and initializing there here doesn't seem to make sense to me. should it be in imfs_init.c?

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125632

 >    (LAST_TRIPLY_INDIRECT * IMFS_MEMFILE_BYTES_PER_BLOCK)
 >  
 > +

avoid making random whitespace changes.

this is incorrectly adding a blank line.

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125633

 > +  IMFS_memfile_allocator allocator;
 > +  IMFS_memfile_deallocator deallocator;
 > +  IMFS_memfile_free_space free_space;

functions should have "verb" names, so more like "allocate, deallocate, get_free_space"

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125634

 > +void *default_allocator(void);
 > +void default_deallocator(void *);
 > +size_t default_free_space(void);

there are in the global namespace. they should have names protected by the scope here, e.g., IMFS_default_allocator.

--
  
Gedare Bloom started a new discussion on cpukit/libfs/src/imfs/imfs_init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125635

 >  
 > +void IMFS_memfile_ops_init(void){
 > +  if(!imfs_memfile_ops.allocator || !imfs_memfile_ops.deallocator || !imfs_memfile_ops.free_space){

check format pipeline, this line looks too long to me.

--
  
Gedare Bloom started a new discussion on cpukit/libfs/src/imfs/imfs_init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125636

 >  
 > +void IMFS_memfile_ops_init(void){
 > +  if(!imfs_memfile_ops.allocator || !imfs_memfile_ops.deallocator || !imfs_memfile_ops.free_space){

is it correct to initialize all of them to the default, even if two of them have been initialized to something else?

--
  
Gedare Bloom started a new discussion on cpukit/libfs/src/imfs/imfs_init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125637

 >  };
 >  
 > +void IMFS_memfile_ops_init(void){

this function is local to this file, it could be `static`?

--
  
Gedare Bloom started a new discussion on cpukit/libfs/src/imfs/imfs_memfile.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125638

 >   *  Prototypes of private routines
 >   */
 > +const IMFS_memfile_ops imfs_memfile_default_ops = {

this doesn't match the comment above it

--
  
Gedare Bloom started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125639

 > +
 > +const char rtems_test_name[] = "PSXIMFS 3";
 > +const char memory[1000000000];

Am I right, this is 953 megabytes? Why using so much?

--
  
Gedare Bloom started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_125640

 > +  freelist = (char *)blk;
 > +
 > +}

I don't understand what these allocator/deallocator functions are doing. It doesn't seem to be correct to me. 

I don't know what is trying to be done with this test case.


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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20250630/c57a6df9/attachment-0001.htm>