Status of 'blacklist' patch?
Tom Tromey
tromey@redhat.com
Fri Oct 28 18:18:00 GMT 2011
>>>>> "Justin" == Justin Lebar <justin.lebar@gmail.com> writes:
Justin> Thanks for the testing advice; I ran the test many times on my
Justin> machine, and it now works without error every time.
Justin> New roll-up patch is attached. The only difference from v4 is
Justin> in skip.exp.
Thanks for persevering.
Your test cases seem nicely comprehensive; thanks for making the extra
effort.
I think one more iteration; I would approve-it-with-changes but there is
one non-cosmetic issue. It isn't too major.
Justin> +/* Attempt to determine architecture of location identified by SAL. */
Justin> +extern struct gdbarch *
Justin> +get_sal_arch (struct symtab_and_line sal);
In a case like this, indent the continuation line a bit.
Justin> + TRY_CATCH (decode_exception, NOT_FOUND_ERROR)
The second argument to TRY_CATCH actually must be one of the
RETURN_MASK_* constants. Here I suggest RETURN_MASK_ERROR.
Justin> + if (decode_exception.reason < 0)
Justin> + {
Then here you can:
if (decode_exception.reason < 0)
{
if (decode_exception.error != NOT_FOUND_ERROR)
throw_exception (decode_exception);
...
Justin> + ALL_SKIPLIST_ENTRIES (e)
Justin> + if (arg == 0 || number_is_in_list(arg, e->number))
Space before the open paren.
Justin> + if (arg != 0 && !number_is_in_list(arg, e->number))
Likewise.
This appears in a few more places -- all uses of number_is_in_list.
Justin> +/* Create a skiplist entry for the given pc corresponding to the given
Justin> + function name and add it to the list. */
Justin> +static void
Justin> +skip_function_pc (CORE_ADDR pc, char *name, struct gdbarch *arch,
Blank line between intro comment and the start of the function.
I think there are a few instances of this.
Justin> +gdb_start
Justin> +gdb_load ${binfile_main}
Use clean_restart here instead.
Justin> +gdb_test "skip file ${srcfile_lib}" \
Justin> +"File ${srcfile_lib} will be skipped when stepping." \
Justin> +"ignoring file in solib" \
Justin> +"No source file named ${srcfile_lib}.*
Justin> +Ignore file pending future shared library load.*"\
Justin> +"y"
Code like this is more readable if you indent the continuation lines.
I realize this can't be done for every line here, and that is ok -- but
the lines following a "\" should be indented.
There should be a space before the last "\".
Justin> +gdb_test "info skip" \
Justin> +"Num\\s+Type\\s+Enb\\s+Address\\s+What\\s*
Justin> +1\\s+file\\s+y\\s+\\s+${srcfile_lib} \\(PENDING\\)\\s*" \
Justin> +"info skip with pending file"
Indentation.
Justin> +gdb_test "info skip" \
Justin> +"Num\\s+Type\\s+Enb\\s+Address\\s+What\\s*
Justin> +1\\s+file\\s+y\\s+\\s+.*${srcfile_lib}\\s*" \
Justin> +"info skip with pending file"
Indentation.
There are a few more of these.
Justin> +gdb_exit
Justin> +gdb_start
Justin> +gdb_load ${binfile_main}
clean_restart
Tom
More information about the Gdb-patches
mailing list