Re: [PATCH 2/2] Add support for GNAT Ravenscar run-time library.

Pedro Alves
2010-09-23T19:49:51Z

On a cursory look, here's what I noted, minus issues already
mentioned:

> +static const char *running_thread_name = "running_thread";

static const char running_thread_name[] = "running_thread";

> + int buf_size = register_size (gdbarch, regnum);
> + char buf [buf_size];

This is not valid C90. Use alloca or MAX_REGISTER_SIZE.

> + && strcmp (target_shortname, ravenscar_ops.to_shortname) != 0)

Please no target name comparisions. You want "is the ravenscar open, even
if something else is on top", not "is the ravenscar target open
and the topmost".

> +/* Observer on inferior_created: push ravenscar thread stratum if needed. */
> +
> +static void
> +ravenscar_inferior_created (struct target_ops *target, int from_tty)
> +{
> + if (ravenscar_task_support
> + && has_ravenscar_runtime ()
> + && strcmp (target_shortname, ravenscar_ops.to_shortname) != 0)
> + ravenscar_initialize (NULL, 0);
> +}

Note that this assumes that the user connects with symbols already
loaded. If you connect, and then do "file foo", ravenscar will not
end pushed.

> + ravenscar_ops.to_shortname = "ravenscar";
> + ravenscar_ops.to_longname = "Ravenscar tasks.";
> + ravenscar_ops.to_doc = "Ravenscar tasks support.";
> + ravenscar_ops.to_wait = ravenscar_wait;

Please no vertical alignment.

> +
> + add_setshow_boolean_cmd ("task-switching", class_obscure,
> + &ravenscar_task_support, _("\
> +Enable or disable support for GNAT Ravenscar tasks"), _("\
> +Show whether supported for GNAT Ravenscar tasks is enabled"),
> + _("\
> +Enable or disable support for task/thread switching\n\
> +with the GNAT Ravenscar run-time lib for bareboard configuration."),
> + NULL, NULL, &set_ravenscar_list,
> + &show_ravenscar_list);

Please implement show_func callbacks for all new set/show commands.

--
Pedro Alves