Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86 was reviewed by Joel Sherrill
-- Joel Sherrill commented on a discussion on cpukit/libmisc/stackchk/check.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108702 > ); > - Stack_check_report_blown_task( heir, pattern_ok ); > + _Stack_checker_reporter_initialize( heir, pattern_ok ); It just occurred to me what might help. Using "initialize" in the name of the type or function name is bad and confusing. All names I suggest likely could still be better but it is a step in the right direction. - `_Stack_checker_reporter_initialize`is a bad name for the reporter function or type. - `_Stack_checker_Reporter_handler` is better name for the type. - `_Stack_checker_Reporter` is a better name for the variable configured. - Eventually providing the current blown task handler and a quieter alternative would be useful. Both will be prototyped in stackchk.h and implemented in check.c. Further - `_Stack_checker_Reporter_print_details` is a better name for the current reporter function. - `_Stack_checker_Reporter_quiet` can be an alternative reporter implementation that JUST calls fatal error. Notice the use of _ and a capital letter to indicate grouping and item within that group. In C++, the variable might be Stack_Checker::Reporter. -- Joel Sherrill started a new discussion on cpukit/include/rtems/confdefs/extensions.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108703 > #include <rtems/score/userextimpl.h> > #include <rtems/sysinit.h> > +#include <rtems/confdefs/inittask.h> Why was this needed? -- Joel Sherrill started a new discussion on cpukit/include/rtems/confdefs/percpu.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108704 > > -#endif /* _RTEMS_CONFDEFS_PERCPU_H */ > +#endif /* _RTEMS_CONFDEFS_PERCPU_H */ What changed here and why is there no newline at the end of the file? -- Joel Sherrill started a new discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108705 > + * still valid or not. > + */ > +typedef void (*Stack_checker_reporter_extension)( Put each parameter on its own line and ) on its own. See function prototypes above and follow them. -- Joel Sherrill started a new discussion on cpukit/libmisc/stackchk/check.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108706 > } > > +void rtems_stack_check_report_blown_task_custom( This should be provided by the user application. It does not belong here. It could be part of your stackchk test to show it is possible. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108707 > +#ifdef HAVE_CONFIG_H File header including SPDX, Doxygen, and license. Should be plenty of examples to copy from. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108708 > + */ > +#define CONFIGURE_STACK_CHECKER_ENABLED > + No need for blank lines between configuration parameters. Use blanks to group similar ones thouogh. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108709 > +#define CONFIGURE_STACK_CHECKER_ENABLED > + > +#define CONFIGURE_STACK_CHECKER_REPORTER This should be defining the name of a custom reporter function which is part of this test. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108710 > +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE > + > + No double blank lines. No blanks between includes unless changing from POSIX ones to RTEMS ones to application specific one. Again, blank lines to indicate groups. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/stackchk03.doc: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108711 > + > +directives: > + + rtems_stack_checker_switch_extention Spelling. -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/stackchk03.doc: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108712 > +# POSSIBILITY OF SUCH DAMAGE. > +# > +This file describes the directives and concepts tested by this test set. Blank line between license and first text -- Joel Sherrill started a new discussion on testsuites/libtests/stackchk03/stackchk03.scn: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108713 > +*** TEST STACKCHK03 *** > + No need for blank lines in this output -- Joel Sherrill commented on a discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108714 > + * still valid or not. > + */ > +typedef void (*Stack_checker_reporter_initialize)(const Thread_Control *, > bool); I think I noted that this should be formatted like a function prototype. -- Joel Sherrill commented on a discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108715 > + * still valid or not > + */ > +void rtems_custom_stack_check_reporter( The stack checker should have a custom checker. The user application/test will provide that and it can be named anything. Yours could be "stackchk03_blown_stack_reporter" and it should work. -- Joel Sherrill commented on a discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86#note_108716 > + * Application provided via <rtems/confdefs.h> > + */ > +extern const Stack_checker_reporter_initialize > _Stack_checker_reporter_initialize; This is still showing the type as `Stack_checker_reporter_initialize`. Hopefully that's just a gitlab artifact. I thought I saw this renamed. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/86 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
