RTEMS | Draft: IMFS User allocator Plugin Code (!534)

[Joel Sherrill (@joel)]

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

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127127

 > +#define FILE_NAME "biggie"
 > +
 > +

Should only have 1 blank line.

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127128

 >   *  @endcode
 >   */
 > +struct IMFS_memfile_ops_h;

You have added this type definition above the definition of IMFS_MEMFILE_DEFAULT_BYTES_PER_BLOCK. Doxygen needs the comments for a variable, function, constant, etc. directly above. Move these below the definition of IMFS_MEMFILE_DEFAULT_BYTES_PER_BLOCK.

Then add a comment block about this type.

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127129

 > +/*
 > + * The allocator function pointer
 > +*/

This should be indented where the * line up.

Also this is not a Doxygen comment. Should be like /** at the beginning and include a more informative description. This is a specific type of allocator function which is associated with the iMFS. Help the next person who is through this code.

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127130

 > +);
 > +
 > +/*

Review and fix from here down for Doxygen comments as described in previous comment.

Note that each element of the structure can be preceded by a Doxygen comment.

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127131

 > +  IMFS_memfile_allocator allocate;
 > +  IMFS_memfile_deallocator deallocate;
 > +  IMFS_memfile_free_space get_free_space; /* for statvfs f_bfree field */

This comment should be part of a Doxygen comment preceding the element.

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/imfs.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127132

 > +};
 > +
 > +void *IMFS_default_allocator(void);

These need Doxygen.

--
  
Joel Sherrill commented on a discussion on cpukit/libfs/src/imfs/imfs_initsupp.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127133

 > +    use_default_ops |= (configure_imfs_memfile_ops.deallocate == NULL);
 > +    use_default_ops |= (configure_imfs_memfile_ops.get_free_space == NULL);
 > +  #endif

OK. Change to _Assert() with one for each element of the structure. Put it in the normal IMFS initialisation function.

Then this function is unused and can be removed.

--
  
Joel Sherrill started a new discussion on cpukit/libfs/src/imfs/imfs_memfile.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127134

 >  #include <string.h>
 >  
 > +extern IMFS_memfile_ops imfs_memfile_ops;

The type and variable name are the same but cased differently. @opticron Any suggestions other than perhaps adding _table or _t to the type name?

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127135

 > +{
 > +  block *blk = (block *)memory;
 > +  if(!blk) return;

Make sure "if(" does appear anywhere. And fix this like the previous suggestion with { return; } on subsequent lines.

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127136

 > +  char *last = memory + sizeof(memory);
 > +
 > +  while(first + 1 <= last){

Formatting. 

while( to while (

space between ){

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psximfs03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127137

 > +}
 > +
 > +void open_it(bool readOnly, bool create)

Did you copy all this from another test? Duplicating is bad. It should be possible to reuse this code without copying it.

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psximfs03/psximfs03.scn: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/534#note_127138

 > +Total written = 1280
 > +close(biggie) - OK 
 > +108 

Why did this print? What does it mean?


-- 
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/20250723/6af5e190/attachment-0001.htm>