[patch 4/4] hw watchpoints made multi-inferior
Yao Qi
yao@codesourcery.com
Tue Dec 7 05:02:00 GMT 2010
On 12/06/2010 07:14 PM, Jan Kratochvil wrote:
Here are some my cents, although I can't approve or reject this patch. :)
> gdb/
> 2010-12-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Fix watchpoints for multi-inferior.
> * breakpoint.c
> (update_watchpoint) <b->pspace != current_program_space>: New.
This changelog entry looks strange. You can describe what is this
change here, like,
* breakpoint.c (update_watchpoint): Return directly if
breakpoint's pspace is not current_program_space.
> * linux-nat.c (linux_nat_iterate_watchpoint_lwps): Fix
> iterate_over_lwps FILTER.
What is FILTER? Why capitalized?
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1302,6 +1302,9 @@ update_watchpoint (struct breakpoint *b, int reparse)
> if (!watchpoint_in_thread_scope (b))
> return;
>
> + if (b->pspace != current_program_space)
> + return;
> +
> /* We don't free locations. They are stored in bp_location array and
> update_global_locations will eventually delete them and remove
> breakpoints if needed. */
> @@ -1879,6 +1882,7 @@ insert_breakpoint_locations (void)
> int val = 0;
> int disabled_breaks = 0;
> int hw_breakpoint_error = 0;
> + struct program_space *saved_current_program_space = current_program_space;
>
> struct ui_file *tmp_error_stream = mem_fileopen ();
> struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream);
> @@ -1908,7 +1912,8 @@ insert_breakpoint_locations (void)
> if we aren't attached to any process yet, we should still
> insert breakpoints. */
> if (!gdbarch_has_global_breakpoints (target_gdbarch)
> - && ptid_equal (inferior_ptid, null_ptid))
> + && (ptid_equal (inferior_ptid, null_ptid)
> + || b->pspace != saved_current_program_space))
> continue;
Please add some comments here for this change. It is not that obvious
to understand without multi-inferior context.
> val = insert_bp_location (b, tmp_error_stream,
> @@ -8265,6 +8270,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
> b = set_raw_breakpoint_without_location (NULL, bp_type);
> set_breakpoint_number (internal, b);
> b->thread = thread;
> + b->pspace = current_program_space;
> b->disposition = disp_donttouch;
> b->exp = exp;
> b->exp_valid_block = exp_valid_block;
> @@ -9404,6 +9410,9 @@ update_global_location_list (int should_insert)
> int keep_in_target = 0;
> int removed = 0;
>
> + if (old_loc->pspace != current_program_space)
> + continue;
code indent problem? Put extra three spaces in front of "continue".
> +static void
> +i386_inferior_data_cleanup (struct inferior *inf, void *arg)
> +{
> + struct i386_inferior_data *inf_data = arg;
> +
> + xfree (inf_data);
> +}
> +
Can't we 'xfree (arg);' directly?
> @@ -1287,7 +1287,7 @@ linux_nat_iterate_watchpoint_lwps
> if (inf->pid == inferior_pid)
> {
> /* Standard mode. */
> - iterate_over_lwps (minus_one_ptid,
> + iterate_over_lwps (pid_to_ptid (inferior_pid),
> iterate_watchpoint_lwps_callback, &data);
Please add some comments to explain this change "minus_one_ptid ->
pid_to_ptid (inferior_pid)". This change is mentioned in changelog as
"Fix iterate_over_lwps FILTER", but it is still no enough, IMO.
> +
> +gdb_test "add-inferior" "Added inferior 2"
> +gdb_test "inferior 2" "witching to inferior 2 .*"
> +gdb_load $binfile
> +
> +if ![runto_main] {
Please add `perror "Couldn't run ${testfile}"'
> + return
> +}
--
Yao (é½å°§)
More information about the Gdb-patches
mailing list