On Mon, May 23, 2022 at 1:40 AM <tia...@sugon.com> wrote:
>
> From: Tian Ye <tia...@sugon.com>
>
What is the bug this fixes? Is there an open ticket for it?
> ---
> cpukit/libmisc/capture/capture.c | 30 ++++++++++++++++++++----
> cpukit/libmisc/capture/capture_support.c | 8 +++++--
> 2 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/cpukit/libmisc/capture/capture.c
> b/cpukit/libmisc/capture/capture.c
> index 2228d135b8..886b4f62ed 100644
> --- a/cpukit/libmisc/capture/capture.c
> +++ b/cpukit/libmisc/capture/capture.c
> @@ -292,6 +292,23 @@ rtems_capture_find_control (rtems_name name, rtems_id id)
> return control;
> }
>
> +static bool
> +rtems_capture_uninitialize_control (rtems_tcb *tcb, void *arg)
> +{
> + rtems_name name = rtems_build_name(0, 0, 0, 0);
> + rtems_id id;
> +
> + id = tcb->Object.id;
> + if (rtems_capture_task_api (id) != OBJECTS_POSIX_API)
> + rtems_object_get_classic_name (id, &name);
> + if (rtems_capture_match_name_id (((rtems_capture_control *)arg)->name,
> + ((rtems_capture_control *)arg)->id, name, id))
> + {
> + tcb->Capture.control = NULL;
Should this 'return true' to short-circuit the thread_iterate?
> + }
> + return false;
> +}
> +
> /*
> * This function checks if a new control structure matches
> * the given task and sets the control if it does.
> @@ -699,6 +716,7 @@ rtems_capture_close (void)
> {
> rtems_capture_control* delete = control;
> control = control->next;
> + _Thread_Iterate (rtems_capture_uninitialize_control, (void *)delete);
> free (delete);
> }
>
> @@ -789,6 +807,7 @@ rtems_capture_flush (bool prime)
> {
> rtems_interrupt_lock_context lock_context_global;
> uint32_t cpu;
> + uint32_t* cpu_flags;
>
> rtems_interrupt_lock_acquire (&capture_lock_global,
> &lock_context_global);
>
> @@ -800,16 +819,17 @@ rtems_capture_flush (bool prime)
>
> _Thread_Iterate (rtems_capture_flush_tcb, NULL);
>
> - if (prime)
> - capture_flags_global &= ~(RTEMS_CAPTURE_TRIGGERED |
> RTEMS_CAPTURE_OVERFLOW);
> - else
> - capture_flags_global &= ~RTEMS_CAPTURE_OVERFLOW;
> + if (prime) {
> + capture_flags_global &= ~RTEMS_CAPTURE_TRIGGERED;
> + }
I don't know what this (and following) change is doing.
>
> for (cpu=0; cpu < rtems_scheduler_get_processor_maximum(); cpu++) {
> RTEMS_INTERRUPT_LOCK_REFERENCE( lock, &(capture_lock_on_cpu( cpu )) )
> rtems_interrupt_lock_context lock_context_per_cpu;
>
> rtems_interrupt_lock_acquire (lock, &lock_context_per_cpu);
> + cpu_flags = &(capture_flags_on_cpu (cpu));
> + *cpu_flags &= ~RTEMS_CAPTURE_OVERFLOW;
> capture_count_on_cpu(cpu) = 0;
> if (capture_records_on_cpu(cpu).buffer)
> rtems_capture_buffer_flush( &capture_records_on_cpu(cpu) );
> @@ -885,6 +905,8 @@ rtems_capture_watch_del (rtems_name name, rtems_id id)
>
> *prev_control = control->next;
>
> + _Thread_Iterate (rtems_capture_uninitialize_control, (void *)control);
> +
> rtems_interrupt_lock_release (&capture_lock_global, &lock_context);
>
> free (control);
> diff --git a/cpukit/libmisc/capture/capture_support.c
> b/cpukit/libmisc/capture/capture_support.c
> index 5b685479d7..fb4ccfe2d8 100644
> --- a/cpukit/libmisc/capture/capture_support.c
> +++ b/cpukit/libmisc/capture/capture_support.c
> @@ -247,7 +247,7 @@ rtems_capture_print_record_capture(int
> cpu,
> {
> fprintf(stdout,"%2i ", cpu);
> rtems_capture_print_timestamp (rec->time);
> - fprintf (stdout, " %12" PRId32 " ", (uint32_t) diff);
> + fprintf (stdout, " %12" PRIu32 " ", (uint32_t) diff);
> rtems_monitor_dump_id (rec->task_id);
> if (name != NULL)
> {
> @@ -439,7 +439,11 @@ rtems_capture_print_watch_list (void)
> fprintf (stdout, " ");
> rtems_monitor_dump_id (rtems_capture_control_id (control));
> fprintf (stdout, " ");
> - rtems_monitor_dump_name (rtems_capture_control_name (control));
> + char name[5] = "????";
This should be declared at the start of the block/function.
> + if (rtems_capture_control_name(control) != 0) {
> + _Objects_Name_to_string((Objects_Name)rtems_capture_control_name
> (control), false, name, 5);
This line is too long, although there are other style problems
throughout this patch. Hopefully we'll have automation to help that.
Also, I'd prefer sizeof() or a const/define macro value instead of the
hard-coded 5.
> + }
> + fprintf(stdout, "%s", name);
> flags = rtems_capture_control_flags (control);
> fprintf (stdout, " %c%c ",
> rtems_capture_watch_global_on () ? 'g' : '-',
> --
> 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
