* [PATCH 6/6] framefilter quit: New test
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
2015-02-07 14:45 ` [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:56 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 4/6] framefilter quit: Make it exception safe Jan Kratochvil
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
it definitely does not test all the RETURN_MASK_ERROR cases. But it tests at
least two of them.
Jan
gdb/testsuite/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.python/py-framefilter.exp (pagination quit - *): New tests.
---
gdb/testsuite/gdb.python/py-framefilter.exp | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 1a0c750..2f81abe 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -74,6 +74,19 @@ gdb_test "bt full" \
".*#0.*cnuf_dne.*h = 9.*f = 42.*g = 19.*bar = $hex \"Inside block x2\".*d = 15.*e = 14.*foo = $hex \"Inside block\".*str = $hex \"The End\".*st2 = $hex \"Is Near\".*b = 12.*c = 5.*" \
"bt full with filters"
+# Test pagination can be aborted even for frame filters.
+gdb_test_no_output "set height 5" "pagination quit - set height limited"
+foreach bttype [list "bt" "bt full"] {
+ set test "pagination quit - $bttype"
+ gdb_test_multiple "$bttype" $test {
+ -re "$pagination_prompt$" {
+ pass $test
+ }
+ }
+ gdb_test "q" "^q\r\nQuit" "pagination quit - $bttype - q"
+}
+gdb_test_no_output "set height unlimited" "pagination quit - set height unlimited"
+
gdb_continue_to_breakpoint "Backtrace end breakpoint"
# Test set/show
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] framefilter quit: Make it exception safe
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
2015-02-07 14:45 ` [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR Jan Kratochvil
2015-02-07 14:45 ` [PATCH 6/6] framefilter quit: New test Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:45 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 1/6] framefilter quit: Obvious whitespacing fixes Jan Kratochvil
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
this this the key part of the series, possibly the unsafe one.
gdb/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/py-framefilter.c (py_print_frame): Mention RETURN_QUIT in
function comment. Wrap all function that can throw in cleanups.
(gdbpy_apply_frame_filter): Wrap all function that can throw in
cleanups.
---
gdb/python/py-framefilter.c | 61 ++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index ba1b237..c232946 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1003,7 +1003,7 @@ py_print_args (PyObject *filter,
If a frame level has been printed, do not print it again (in the
case of elided frames). Returns EXT_LANG_BT_ERROR on error, with any
GDB exceptions converted to a Python exception, or EXT_LANG_BT_COMPLETED
- on success. */
+ on success. It can also throw an exception RETURN_QUIT. */
static enum ext_lang_bt_status
py_print_frame (PyObject *filter, int flags,
@@ -1014,7 +1014,7 @@ py_print_frame (PyObject *filter, int flags,
CORE_ADDR address = 0;
struct gdbarch *gdbarch = NULL;
struct frame_info *frame = NULL;
- struct cleanup *cleanup_stack = make_cleanup (null_cleanup, NULL);
+ struct cleanup *cleanup_stack;
struct value_print_options opts;
PyObject *py_inf_frame;
int print_level, print_frame_info, print_args, print_locals;
@@ -1033,20 +1033,14 @@ py_print_frame (PyObject *filter, int flags,
read them if they returned filter object requires us to do so. */
py_inf_frame = PyObject_CallMethod (filter, "inferior_frame", NULL);
if (py_inf_frame == NULL)
- {
- do_cleanups (cleanup_stack);
- return EXT_LANG_BT_ERROR;
- }
+ return EXT_LANG_BT_ERROR;
frame = frame_object_to_frame_info (py_inf_frame);;
Py_DECREF (py_inf_frame);
if (frame == NULL)
- {
- do_cleanups (cleanup_stack);
- return EXT_LANG_BT_ERROR;
- }
+ return EXT_LANG_BT_ERROR;
TRY_CATCH (except, RETURN_MASK_ALL)
{
@@ -1055,7 +1049,6 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
@@ -1064,14 +1057,12 @@ py_print_frame (PyObject *filter, int flags,
{
if (py_mi_print_variables (filter, out, &opts,
args_type, frame) == EXT_LANG_BT_ERROR)
- {
- do_cleanups (cleanup_stack);
- return EXT_LANG_BT_ERROR;
- }
- do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
return EXT_LANG_BT_COMPLETED;
}
+ cleanup_stack = make_cleanup (null_cleanup, NULL);
+
/* -stack-list-locals does not require a
wrapping frame attribute. */
if (print_frame_info || (print_args && ! print_locals))
@@ -1179,6 +1170,7 @@ py_print_frame (PyObject *filter, int flags,
if (PyObject_HasAttrString (filter, "function"))
{
PyObject *py_func = PyObject_CallMethod (filter, "function", NULL);
+ struct cleanup *py_func_cleanup;
const char *function = NULL;
if (py_func == NULL)
@@ -1186,6 +1178,7 @@ py_print_frame (PyObject *filter, int flags,
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
+ py_func_cleanup = make_cleanup_py_decref (py_func);
if (gdbpy_is_string (py_func))
{
@@ -1196,7 +1189,6 @@ py_print_frame (PyObject *filter, int flags,
if (function == NULL)
{
- Py_DECREF (py_func);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
@@ -1222,7 +1214,6 @@ py_print_frame (PyObject *filter, int flags,
PyErr_SetString (PyExc_RuntimeError,
_("FrameDecorator.function: expecting a " \
"String, integer or None."));
- Py_DECREF (py_func);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
@@ -1237,12 +1228,11 @@ py_print_frame (PyObject *filter, int flags,
}
if (except.reason < 0)
{
- Py_DECREF (py_func);
gdbpy_convert_exception (except);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
- Py_DECREF (py_func);
+ do_cleanups (py_func_cleanup);
}
}
@@ -1275,12 +1265,14 @@ py_print_frame (PyObject *filter, int flags,
if (PyObject_HasAttrString (filter, "filename"))
{
PyObject *py_fn = PyObject_CallMethod (filter, "filename", NULL);
+ struct cleanup *py_fn_cleanup;
if (py_fn == NULL)
{
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
+ py_fn_cleanup = make_cleanup_py_decref (py_fn);
if (py_fn != Py_None)
{
@@ -1288,7 +1280,6 @@ py_print_frame (PyObject *filter, int flags,
if (filename == NULL)
{
- Py_DECREF (py_fn);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
@@ -1304,18 +1295,18 @@ py_print_frame (PyObject *filter, int flags,
}
if (except.reason < 0)
{
- Py_DECREF (py_fn);
gdbpy_convert_exception (except);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
}
- Py_DECREF (py_fn);
+ do_cleanups (py_fn_cleanup);
}
if (PyObject_HasAttrString (filter, "line"))
{
PyObject *py_line = PyObject_CallMethod (filter, "line", NULL);
+ struct cleanup *py_line_cleanup;
int line;
if (py_line == NULL)
@@ -1323,6 +1314,7 @@ py_print_frame (PyObject *filter, int flags,
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
+ py_line_cleanup = make_cleanup_py_decref (py_line);
if (py_line != Py_None)
{
@@ -1335,13 +1327,12 @@ py_print_frame (PyObject *filter, int flags,
}
if (except.reason < 0)
{
- Py_DECREF (py_line);
gdbpy_convert_exception (except);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
}
- Py_DECREF (py_line);
+ do_cleanups (py_line_cleanup);
}
}
@@ -1374,6 +1365,7 @@ py_print_frame (PyObject *filter, int flags,
{
PyObject *elided;
+ struct cleanup *elided_cleanup;
/* Finally recursively print elided frames, if any. */
elided = get_py_iter_from_func (filter, "elided");
@@ -1382,8 +1374,8 @@ py_print_frame (PyObject *filter, int flags,
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
+ elided_cleanup = make_cleanup_py_decref (elided);
- make_cleanup_py_decref (elided);
if (elided != Py_None)
{
PyObject *item;
@@ -1395,19 +1387,20 @@ py_print_frame (PyObject *filter, int flags,
while ((item = PyIter_Next (elided)))
{
+ struct cleanup *item_cleanup = make_cleanup_py_decref (item);
+
enum ext_lang_bt_status success = py_print_frame (item, flags,
args_type, out,
indent,
levels_printed);
+ do_cleanups (item_cleanup);
+
if (success == EXT_LANG_BT_ERROR)
{
- Py_DECREF (item);
do_cleanups (cleanup_stack);
return EXT_LANG_BT_ERROR;
}
-
- Py_DECREF (item);
}
if (item == NULL && PyErr_Occurred ())
{
@@ -1415,8 +1408,8 @@ py_print_frame (PyObject *filter, int flags,
return EXT_LANG_BT_ERROR;
}
}
- }
-
+ do_cleanups (elided_cleanup);
+ }
do_cleanups (cleanup_stack);
return EXT_LANG_BT_COMPLETED;
@@ -1569,15 +1562,17 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
while ((item = PyIter_Next (iterable)))
{
+ struct cleanup *item_cleanup = make_cleanup_py_decref (item);
+
success = py_print_frame (item, flags, args_type, out, 0,
levels_printed);
+ do_cleanups (item_cleanup);
+
/* Do not exit on error printing a single frame. Print the
error and continue with other frames. */
if (success == EXT_LANG_BT_ERROR)
gdbpy_print_stack ();
-
- Py_DECREF (item);
}
if (item == NULL && PyErr_Occurred ())
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/6] framefilter quit: PR cli/17716
@ 2015-02-07 14:45 Jan Kratochvil
2015-02-07 14:45 ` [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR Jan Kratochvil
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi Phil,
as I have seen gdb-7.9 may get released with this annoying bug I have attempted
a fix. It is far from complete but it should hopefully handle most of the
uninterruptable cases - although some of them will apparently still remain
there.
Primarily I do not understand intentions of the code:
gdbpy_convert_exception() is required in code called from Python, such as for
bpfinishpy_init() as one of many examples. But if I haven't make mistake I do
not see any of the GDB functions changed in this patchset (py_print_single_arg,
enumerate_locals, py_print_frame) would be called by Python. Therefore I do
not see why they should call gdbpy_convert_exception() at all (moreover they
call it so many times). These functions are normally called by GDB and GDB is
GDB exception safe so GDB exceptions can be safely thrown. Not all of the
gdb/python/ code was GDB exception safe, this is why I had to change for
example also gdbpy_apply_frame_filter(). I find more safe to write code as
exception safe in general. Hopefully I did not forget to make exception safe
any of the possible callers of the involved functions.
Then I do not understand why there is EXT_LANG_BT_ERROR. GDB has exceptions so
it does not need error return values. Error return values can be used in cases
when it is more simple for the code (As current implementation of GDB
exceptions is needlessly complicated to use) - when callee detects a problem on
its own. But if caller has to TRY_CATCH exceptions from futher callees and
convert them into EXT_LANG_BT_ERROR so that its caller can check for
EXT_LANG_BT_ERROR... Why hasn't the caller just do TRY_CATCH on its own?
I haven't tried to remove EXT_LANG_BT_ERROR at all myself in this patchset,
though. That would be larger work.
As I said the patch is not complete but I believe the direction of this patch
is correct and futher fixes can extend it.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] framefilter quit: Obvious whitespacing fixes
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
` (2 preceding siblings ...)
2015-02-07 14:45 ` [PATCH 4/6] framefilter quit: Make it exception safe Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:37 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos Jan Kratochvil
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
planned to be checked in with the series.
Jan
gdb/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/py-framefilter.c (py_print_frame): Whitespacing fixes.
---
gdb/python/py-framefilter.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 97dce89..c1c2653 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1052,7 +1052,6 @@ py_print_frame (PyObject *filter, int flags,
goto error;
}
-
/* stack-list-variables. */
if (print_locals && print_args && ! print_frame_info)
{
@@ -1077,15 +1076,15 @@ py_print_frame (PyObject *filter, int flags,
and are printed with indention. */
if (indent > 0)
{
- TRY_CATCH (except, RETURN_MASK_ALL)
- {
- ui_out_spaces (out, indent*4);
- }
- if (except.reason < 0)
- {
- gdbpy_convert_exception (except);
- goto error;
- }
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ ui_out_spaces (out, indent*4);
+ }
+ if (except.reason < 0)
+ {
+ gdbpy_convert_exception (except);
+ goto error;
+ }
}
/* The address is required for frame annotations, and also for
@@ -1175,7 +1174,7 @@ py_print_frame (PyObject *filter, int flags,
if (gdbpy_is_string (py_func))
{
- char *function_to_free = NULL;
+ char *function_to_free;
function = function_to_free =
python_string_to_host_string (py_func);
@@ -1208,7 +1207,6 @@ py_print_frame (PyObject *filter, int flags,
goto error;
}
-
TRY_CATCH (except, RETURN_MASK_ALL)
{
annotate_frame_function_name ();
@@ -1254,8 +1252,8 @@ py_print_frame (PyObject *filter, int flags,
if (PyObject_HasAttrString (filter, "filename"))
{
- PyObject *py_fn = PyObject_CallMethod (filter, "filename",
- NULL);
+ PyObject *py_fn = PyObject_CallMethod (filter, "filename", NULL);
+
if (py_fn != NULL)
{
if (py_fn != Py_None)
@@ -1344,7 +1342,7 @@ py_print_frame (PyObject *filter, int flags,
}
/* Finally recursively print elided frames, if any. */
- elided = get_py_iter_from_func (filter, "elided");
+ elided = get_py_iter_from_func (filter, "elided");
if (elided == NULL)
goto error;
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:52 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 6/6] framefilter quit: New test Jan Kratochvil
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
now when the code is exception safe we can let RETURN_QUIT to pass through as
all the installed cleanups with handle that.
Jan
gdb/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/py-framefilter.c (py_print_single_arg, enumerate_locals)
(py_print_frame): Use RETURN_MASK_ERROR.
---
gdb/python/py-framefilter.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index c232946..5531d2b 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -376,7 +376,7 @@ py_print_single_arg (struct ui_out *out,
else
val = fv;
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
@@ -761,7 +761,7 @@ enumerate_locals (PyObject *iter,
/* If the object did not provide a value, read it. */
if (val == NULL)
{
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
val = read_var_value (sym, frame);
}
@@ -781,7 +781,7 @@ enumerate_locals (PyObject *iter,
if (print_args_field || args_type != NO_VALUES)
make_cleanup_ui_out_tuple_begin_end (out, NULL);
}
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
if (! ui_out_is_mi_like_p (out))
{
@@ -838,7 +838,7 @@ enumerate_locals (PyObject *iter,
do_cleanups (locals_cleanups);
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
ui_out_text (out, "\n");
}
@@ -1042,7 +1042,7 @@ py_print_frame (PyObject *filter, int flags,
if (frame == NULL)
return EXT_LANG_BT_ERROR;
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
gdbarch = get_frame_arch (frame);
}
@@ -1074,7 +1074,7 @@ py_print_frame (PyObject *filter, int flags,
and are printed with indention. */
if (indent > 0)
{
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
ui_out_spaces (out, indent*4);
}
@@ -1117,7 +1117,7 @@ py_print_frame (PyObject *filter, int flags,
slot = (struct frame_info **) htab_find_slot (levels_printed,
frame, INSERT);
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
level = frame_relative_level (frame);
@@ -1151,7 +1151,7 @@ py_print_frame (PyObject *filter, int flags,
print nothing. */
if (opts.addressprint && has_addr)
{
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
annotate_frame_address ();
ui_out_field_core_addr (out, "addr", gdbarch, address);
@@ -1218,7 +1218,7 @@ py_print_frame (PyObject *filter, int flags,
return EXT_LANG_BT_ERROR;
}
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
annotate_frame_function_name ();
if (function == NULL)
@@ -1251,7 +1251,7 @@ py_print_frame (PyObject *filter, int flags,
/* File name/source/line number information. */
if (print_frame_info)
{
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
annotate_frame_source_begin ();
}
@@ -1285,7 +1285,7 @@ py_print_frame (PyObject *filter, int flags,
}
make_cleanup (xfree, filename);
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
ui_out_wrap_hint (out, " ");
ui_out_text (out, " at ");
@@ -1319,7 +1319,7 @@ py_print_frame (PyObject *filter, int flags,
if (py_line != Py_None)
{
line = PyLong_AsLong (py_line);
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
ui_out_text (out, ":");
annotate_frame_source_line ();
@@ -1340,7 +1340,7 @@ py_print_frame (PyObject *filter, int flags,
elided frames, so if MI output detected do not send newline. */
if (! ui_out_is_mi_like_p (out))
{
- TRY_CATCH (except, RETURN_MASK_ALL)
+ TRY_CATCH (except, RETURN_MASK_ERROR)
{
annotate_frame_end ();
ui_out_text (out, "\n");
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
` (3 preceding siblings ...)
2015-02-07 14:45 ` [PATCH 1/6] framefilter quit: Obvious whitespacing fixes Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:43 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 2/6] framefilter quit: Code cleanup: Reindentation Jan Kratochvil
2015-02-10 17:40 ` [PATCH 0/6] framefilter quit: PR cli/17716 Pedro Alves
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
goto error patters are sometimes AFAIK used in C for the cases like:
int retval=-1;
if (!(a=malloc())) goto error;
if (!(b=malloc())) goto error_a;
if (!(c=malloc())) goto error_b;
retval=0;
error_c: free(c);
error_b: free(b);
error_a: free(a);
error: return retval;
But here there is single error label with one do_cleanups() which I do not find
it worth the goto complication. Without goto one can then furher merge code in
the exit paths in the next patches and ... after all it is all the same, just
without a goto.
Jan
gdb/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/py-framefilter.c (py_print_frame): Substitute goto error.
Remove the error label.
---
gdb/python/py-framefilter.c | 103 +++++++++++++++++++++++++++++++------------
1 file changed, 74 insertions(+), 29 deletions(-)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 896e2a8..ba1b237 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1033,14 +1033,20 @@ py_print_frame (PyObject *filter, int flags,
read them if they returned filter object requires us to do so. */
py_inf_frame = PyObject_CallMethod (filter, "inferior_frame", NULL);
if (py_inf_frame == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
frame = frame_object_to_frame_info (py_inf_frame);;
Py_DECREF (py_inf_frame);
if (frame == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
TRY_CATCH (except, RETURN_MASK_ALL)
{
@@ -1049,7 +1055,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
/* stack-list-variables. */
@@ -1057,7 +1064,10 @@ py_print_frame (PyObject *filter, int flags,
{
if (py_mi_print_variables (filter, out, &opts,
args_type, frame) == EXT_LANG_BT_ERROR)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
do_cleanups (cleanup_stack);
return EXT_LANG_BT_COMPLETED;
}
@@ -1080,7 +1090,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
@@ -1091,7 +1102,10 @@ py_print_frame (PyObject *filter, int flags,
PyObject *paddr = PyObject_CallMethod (filter, "address", NULL);
if (paddr == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
if (paddr != Py_None)
{
@@ -1135,7 +1149,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
@@ -1155,7 +1170,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
@@ -1166,7 +1182,10 @@ py_print_frame (PyObject *filter, int flags,
const char *function = NULL;
if (py_func == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
if (gdbpy_is_string (py_func))
{
@@ -1178,7 +1197,8 @@ py_print_frame (PyObject *filter, int flags,
if (function == NULL)
{
Py_DECREF (py_func);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
make_cleanup (xfree, function_to_free);
}
@@ -1188,7 +1208,10 @@ py_print_frame (PyObject *filter, int flags,
struct bound_minimal_symbol msymbol;
if (PyErr_Occurred ())
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
msymbol = lookup_minimal_symbol_by_pc (addr);
if (msymbol.minsym != NULL)
@@ -1200,7 +1223,8 @@ py_print_frame (PyObject *filter, int flags,
_("FrameDecorator.function: expecting a " \
"String, integer or None."));
Py_DECREF (py_func);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
TRY_CATCH (except, RETURN_MASK_ALL)
@@ -1215,7 +1239,8 @@ py_print_frame (PyObject *filter, int flags,
{
Py_DECREF (py_func);
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
Py_DECREF (py_func);
}
@@ -1227,7 +1252,10 @@ py_print_frame (PyObject *filter, int flags,
if (print_args)
{
if (py_print_args (filter, out, args_type, frame) == EXT_LANG_BT_ERROR)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
}
/* File name/source/line number information. */
@@ -1240,7 +1268,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
if (PyObject_HasAttrString (filter, "filename"))
@@ -1248,7 +1277,10 @@ py_print_frame (PyObject *filter, int flags,
PyObject *py_fn = PyObject_CallMethod (filter, "filename", NULL);
if (py_fn == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
if (py_fn != Py_None)
{
@@ -1257,7 +1289,8 @@ py_print_frame (PyObject *filter, int flags,
if (filename == NULL)
{
Py_DECREF (py_fn);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
make_cleanup (xfree, filename);
@@ -1273,7 +1306,8 @@ py_print_frame (PyObject *filter, int flags,
{
Py_DECREF (py_fn);
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
Py_DECREF (py_fn);
@@ -1285,7 +1319,10 @@ py_print_frame (PyObject *filter, int flags,
int line;
if (py_line == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
if (py_line != Py_None)
{
@@ -1300,7 +1337,8 @@ py_print_frame (PyObject *filter, int flags,
{
Py_DECREF (py_line);
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
Py_DECREF (py_line);
@@ -1319,7 +1357,8 @@ py_print_frame (PyObject *filter, int flags,
if (except.reason < 0)
{
gdbpy_convert_exception (except);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
}
@@ -1327,7 +1366,10 @@ py_print_frame (PyObject *filter, int flags,
{
if (py_print_locals (filter, out, args_type, indent,
frame) == EXT_LANG_BT_ERROR)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
}
{
@@ -1336,7 +1378,10 @@ py_print_frame (PyObject *filter, int flags,
/* Finally recursively print elided frames, if any. */
elided = get_py_iter_from_func (filter, "elided");
if (elided == NULL)
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
make_cleanup_py_decref (elided);
if (elided != Py_None)
@@ -1358,23 +1403,23 @@ py_print_frame (PyObject *filter, int flags,
if (success == EXT_LANG_BT_ERROR)
{
Py_DECREF (item);
- goto error;
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
}
Py_DECREF (item);
}
if (item == NULL && PyErr_Occurred ())
- goto error;
+ {
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_ERROR;
+ }
}
}
do_cleanups (cleanup_stack);
return EXT_LANG_BT_COMPLETED;
-
- error:
- do_cleanups (cleanup_stack);
- return EXT_LANG_BT_ERROR;
}
/* Helper function to initiate frame filter invocation at starting
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] framefilter quit: Code cleanup: Reindentation
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
` (4 preceding siblings ...)
2015-02-07 14:45 ` [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos Jan Kratochvil
@ 2015-02-07 14:45 ` Jan Kratochvil
2015-02-11 13:39 ` [commit+7.9] " Jan Kratochvil
2015-02-10 17:40 ` [PATCH 0/6] framefilter quit: PR cli/17716 Pedro Alves
6 siblings, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-07 14:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
Hi,
nothing significant but I find code more clear with less deep indentation.
Providing also more readable diff with -b.
Jan
gdb/ChangeLog
2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/py-framefilter.c (py_print_frame): Put conditional code paths
with goto first, indent the former else codepath left. Put variable
'elided' to a new inner block.
#--- a/gdb/python/py-framefilter.c
#+++ b/gdb/python/py-framefilter.c
#@@ -1016,7 +1016,7 @@ py_print_frame (PyObject *filter, int flags,
# struct frame_info *frame = NULL;
# struct cleanup *cleanup_stack = make_cleanup (null_cleanup, NULL);
# struct value_print_options opts;
#- PyObject *py_inf_frame, *elided;
#+ PyObject *py_inf_frame;
# int print_level, print_frame_info, print_args, print_locals;
# volatile struct gdb_exception except;
#
#@@ -1058,12 +1058,9 @@ py_print_frame (PyObject *filter, int flags,
# if (py_mi_print_variables (filter, out, &opts,
# args_type, frame) == EXT_LANG_BT_ERROR)
# goto error;
#- else
#- {
# do_cleanups (cleanup_stack);
# return EXT_LANG_BT_COMPLETED;
# }
#- }
#
# /* -stack-list-locals does not require a
# wrapping frame attribute. */
#@@ -1092,8 +1089,10 @@ py_print_frame (PyObject *filter, int flags,
# if (PyObject_HasAttrString (filter, "address"))
# {
# PyObject *paddr = PyObject_CallMethod (filter, "address", NULL);
#- if (paddr != NULL)
#- {
#+
#+ if (paddr == NULL)
#+ goto error;
#+
# if (paddr != Py_None)
# {
# address = PyLong_AsLong (paddr);
#@@ -1101,9 +1100,6 @@ py_print_frame (PyObject *filter, int flags,
# }
# Py_DECREF (paddr);
# }
#- else
#- goto error;
#- }
# }
#
# /* Print frame level. MI does not require the level if
#@@ -1167,11 +1163,11 @@ py_print_frame (PyObject *filter, int flags,
# if (PyObject_HasAttrString (filter, "function"))
# {
# PyObject *py_func = PyObject_CallMethod (filter, "function", NULL);
#-
#- if (py_func != NULL)
#- {
# const char *function = NULL;
#
#+ if (py_func == NULL)
#+ goto error;
#+
# if (gdbpy_is_string (py_func))
# {
# char *function_to_free;
#@@ -1223,9 +1220,6 @@ py_print_frame (PyObject *filter, int flags,
# }
# Py_DECREF (py_func);
# }
#- else
#- goto error;
#- }
# }
#
#
#@@ -1254,8 +1248,9 @@ py_print_frame (PyObject *filter, int flags,
# {
# PyObject *py_fn = PyObject_CallMethod (filter, "filename", NULL);
#
#- if (py_fn != NULL)
#- {
#+ if (py_fn == NULL)
#+ goto error;
#+
# if (py_fn != Py_None)
# {
# char *filename = python_string_to_host_string (py_fn);
#@@ -1284,17 +1279,15 @@ py_print_frame (PyObject *filter, int flags,
# }
# Py_DECREF (py_fn);
# }
#- else
#- goto error;
#- }
#
# if (PyObject_HasAttrString (filter, "line"))
# {
# PyObject *py_line = PyObject_CallMethod (filter, "line", NULL);
# int line;
#
#- if (py_line != NULL)
#- {
#+ if (py_line == NULL)
#+ goto error;
#+
# if (py_line != Py_None)
# {
# line = PyLong_AsLong (py_line);
#@@ -1313,9 +1306,6 @@ py_print_frame (PyObject *filter, int flags,
# }
# Py_DECREF (py_line);
# }
#- else
#- goto error;
#- }
# }
#
# /* For MI we need to deal with the "children" list population of
#@@ -1341,6 +1331,9 @@ py_print_frame (PyObject *filter, int flags,
# goto error;
# }
#
#+ {
#+ PyObject *elided;
#+
# /* Finally recursively print elided frames, if any. */
# elided = get_py_iter_from_func (filter, "elided");
# if (elided == NULL)
#@@ -1374,6 +1367,7 @@ py_print_frame (PyObject *filter, int flags,
# if (item == NULL && PyErr_Occurred ())
# goto error;
# }
#+ }
#
#
# do_cleanups (cleanup_stack);
---
gdb/python/py-framefilter.c | 259 +++++++++++++++++++++----------------------
1 file changed, 126 insertions(+), 133 deletions(-)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index c1c2653..896e2a8 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1016,7 +1016,7 @@ py_print_frame (PyObject *filter, int flags,
struct frame_info *frame = NULL;
struct cleanup *cleanup_stack = make_cleanup (null_cleanup, NULL);
struct value_print_options opts;
- PyObject *py_inf_frame, *elided;
+ PyObject *py_inf_frame;
int print_level, print_frame_info, print_args, print_locals;
volatile struct gdb_exception except;
@@ -1058,11 +1058,8 @@ py_print_frame (PyObject *filter, int flags,
if (py_mi_print_variables (filter, out, &opts,
args_type, frame) == EXT_LANG_BT_ERROR)
goto error;
- else
- {
- do_cleanups (cleanup_stack);
- return EXT_LANG_BT_COMPLETED;
- }
+ do_cleanups (cleanup_stack);
+ return EXT_LANG_BT_COMPLETED;
}
/* -stack-list-locals does not require a
@@ -1092,17 +1089,16 @@ py_print_frame (PyObject *filter, int flags,
if (PyObject_HasAttrString (filter, "address"))
{
PyObject *paddr = PyObject_CallMethod (filter, "address", NULL);
- if (paddr != NULL)
+
+ if (paddr == NULL)
+ goto error;
+
+ if (paddr != Py_None)
{
- if (paddr != Py_None)
- {
- address = PyLong_AsLong (paddr);
- has_addr = 1;
- }
- Py_DECREF (paddr);
+ address = PyLong_AsLong (paddr);
+ has_addr = 1;
}
- else
- goto error;
+ Py_DECREF (paddr);
}
}
@@ -1167,64 +1163,61 @@ py_print_frame (PyObject *filter, int flags,
if (PyObject_HasAttrString (filter, "function"))
{
PyObject *py_func = PyObject_CallMethod (filter, "function", NULL);
+ const char *function = NULL;
- if (py_func != NULL)
- {
- const char *function = NULL;
-
- if (gdbpy_is_string (py_func))
- {
- char *function_to_free;
-
- function = function_to_free =
- python_string_to_host_string (py_func);
+ if (py_func == NULL)
+ goto error;
- if (function == NULL)
- {
- Py_DECREF (py_func);
- goto error;
- }
- make_cleanup (xfree, function_to_free);
- }
- else if (PyLong_Check (py_func))
- {
- CORE_ADDR addr = PyLong_AsUnsignedLongLong (py_func);
- struct bound_minimal_symbol msymbol;
+ if (gdbpy_is_string (py_func))
+ {
+ char *function_to_free;
- if (PyErr_Occurred ())
- goto error;
+ function = function_to_free =
+ python_string_to_host_string (py_func);
- msymbol = lookup_minimal_symbol_by_pc (addr);
- if (msymbol.minsym != NULL)
- function = MSYMBOL_PRINT_NAME (msymbol.minsym);
- }
- else if (py_func != Py_None)
+ if (function == NULL)
{
- PyErr_SetString (PyExc_RuntimeError,
- _("FrameDecorator.function: expecting a " \
- "String, integer or None."));
Py_DECREF (py_func);
goto error;
}
+ make_cleanup (xfree, function_to_free);
+ }
+ else if (PyLong_Check (py_func))
+ {
+ CORE_ADDR addr = PyLong_AsUnsignedLongLong (py_func);
+ struct bound_minimal_symbol msymbol;
- TRY_CATCH (except, RETURN_MASK_ALL)
- {
- annotate_frame_function_name ();
- if (function == NULL)
- ui_out_field_skip (out, "func");
- else
- ui_out_field_string (out, "func", function);
- }
- if (except.reason < 0)
- {
- Py_DECREF (py_func);
- gdbpy_convert_exception (except);
- goto error;
- }
+ if (PyErr_Occurred ())
+ goto error;
+
+ msymbol = lookup_minimal_symbol_by_pc (addr);
+ if (msymbol.minsym != NULL)
+ function = MSYMBOL_PRINT_NAME (msymbol.minsym);
+ }
+ else if (py_func != Py_None)
+ {
+ PyErr_SetString (PyExc_RuntimeError,
+ _("FrameDecorator.function: expecting a " \
+ "String, integer or None."));
Py_DECREF (py_func);
+ goto error;
}
- else
- goto error;
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ annotate_frame_function_name ();
+ if (function == NULL)
+ ui_out_field_skip (out, "func");
+ else
+ ui_out_field_string (out, "func", function);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (py_func);
+ gdbpy_convert_exception (except);
+ goto error;
+ }
+ Py_DECREF (py_func);
}
}
@@ -1254,38 +1247,36 @@ py_print_frame (PyObject *filter, int flags,
{
PyObject *py_fn = PyObject_CallMethod (filter, "filename", NULL);
- if (py_fn != NULL)
+ if (py_fn == NULL)
+ goto error;
+
+ if (py_fn != Py_None)
{
- if (py_fn != Py_None)
- {
- char *filename = python_string_to_host_string (py_fn);
+ char *filename = python_string_to_host_string (py_fn);
- if (filename == NULL)
- {
- Py_DECREF (py_fn);
- goto error;
- }
+ if (filename == NULL)
+ {
+ Py_DECREF (py_fn);
+ goto error;
+ }
- make_cleanup (xfree, filename);
- TRY_CATCH (except, RETURN_MASK_ALL)
- {
- ui_out_wrap_hint (out, " ");
- ui_out_text (out, " at ");
- annotate_frame_source_file ();
- ui_out_field_string (out, "file", filename);
- annotate_frame_source_file_end ();
- }
- if (except.reason < 0)
- {
- Py_DECREF (py_fn);
- gdbpy_convert_exception (except);
- goto error;
- }
+ make_cleanup (xfree, filename);
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ ui_out_wrap_hint (out, " ");
+ ui_out_text (out, " at ");
+ annotate_frame_source_file ();
+ ui_out_field_string (out, "file", filename);
+ annotate_frame_source_file_end ();
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (py_fn);
+ gdbpy_convert_exception (except);
+ goto error;
}
- Py_DECREF (py_fn);
}
- else
- goto error;
+ Py_DECREF (py_fn);
}
if (PyObject_HasAttrString (filter, "line"))
@@ -1293,28 +1284,26 @@ py_print_frame (PyObject *filter, int flags,
PyObject *py_line = PyObject_CallMethod (filter, "line", NULL);
int line;
- if (py_line != NULL)
+ if (py_line == NULL)
+ goto error;
+
+ if (py_line != Py_None)
{
- if (py_line != Py_None)
+ line = PyLong_AsLong (py_line);
+ TRY_CATCH (except, RETURN_MASK_ALL)
{
- line = PyLong_AsLong (py_line);
- TRY_CATCH (except, RETURN_MASK_ALL)
- {
- ui_out_text (out, ":");
- annotate_frame_source_line ();
- ui_out_field_int (out, "line", line);
- }
- if (except.reason < 0)
- {
- Py_DECREF (py_line);
- gdbpy_convert_exception (except);
- goto error;
- }
+ ui_out_text (out, ":");
+ annotate_frame_source_line ();
+ ui_out_field_int (out, "line", line);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (py_line);
+ gdbpy_convert_exception (except);
+ goto error;
}
- Py_DECREF (py_line);
}
- else
- goto error;
+ Py_DECREF (py_line);
}
}
@@ -1341,38 +1330,42 @@ py_print_frame (PyObject *filter, int flags,
goto error;
}
- /* Finally recursively print elided frames, if any. */
- elided = get_py_iter_from_func (filter, "elided");
- if (elided == NULL)
- goto error;
+ {
+ PyObject *elided;
- make_cleanup_py_decref (elided);
- if (elided != Py_None)
- {
- PyObject *item;
+ /* Finally recursively print elided frames, if any. */
+ elided = get_py_iter_from_func (filter, "elided");
+ if (elided == NULL)
+ goto error;
- make_cleanup_ui_out_list_begin_end (out, "children");
+ make_cleanup_py_decref (elided);
+ if (elided != Py_None)
+ {
+ PyObject *item;
- if (! ui_out_is_mi_like_p (out))
- indent++;
+ make_cleanup_ui_out_list_begin_end (out, "children");
- while ((item = PyIter_Next (elided)))
- {
- enum ext_lang_bt_status success = py_print_frame (item, flags,
- args_type, out,
- indent,
- levels_printed);
+ if (! ui_out_is_mi_like_p (out))
+ indent++;
- if (success == EXT_LANG_BT_ERROR)
- {
- Py_DECREF (item);
- goto error;
- }
+ while ((item = PyIter_Next (elided)))
+ {
+ enum ext_lang_bt_status success = py_print_frame (item, flags,
+ args_type, out,
+ indent,
+ levels_printed);
- Py_DECREF (item);
- }
- if (item == NULL && PyErr_Occurred ())
- goto error;
+ if (success == EXT_LANG_BT_ERROR)
+ {
+ Py_DECREF (item);
+ goto error;
+ }
+
+ Py_DECREF (item);
+ }
+ if (item == NULL && PyErr_Occurred ())
+ goto error;
+ }
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] framefilter quit: PR cli/17716
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
` (5 preceding siblings ...)
2015-02-07 14:45 ` [PATCH 2/6] framefilter quit: Code cleanup: Reindentation Jan Kratochvil
@ 2015-02-10 17:40 ` Pedro Alves
2015-02-10 19:19 ` Phil Muldoon
6 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2015-02-10 17:40 UTC (permalink / raw)
To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon
Hi Jan,
I read this, and it looks pretty good to me.
Phil, any comments?
Thanks,
Pedro Alves
On 02/07/2015 02:45 PM, Jan Kratochvil wrote:
> Hi Phil,
>
> as I have seen gdb-7.9 may get released with this annoying bug I have attempted
> a fix. It is far from complete but it should hopefully handle most of the
> uninterruptable cases - although some of them will apparently still remain
> there.
>
> Primarily I do not understand intentions of the code:
>
> gdbpy_convert_exception() is required in code called from Python, such as for
> bpfinishpy_init() as one of many examples. But if I haven't make mistake I do
> not see any of the GDB functions changed in this patchset (py_print_single_arg,
> enumerate_locals, py_print_frame) would be called by Python. Therefore I do
> not see why they should call gdbpy_convert_exception() at all (moreover they
> call it so many times). These functions are normally called by GDB and GDB is
> GDB exception safe so GDB exceptions can be safely thrown. Not all of the
> gdb/python/ code was GDB exception safe, this is why I had to change for
> example also gdbpy_apply_frame_filter(). I find more safe to write code as
> exception safe in general. Hopefully I did not forget to make exception safe
> any of the possible callers of the involved functions.
>
> Then I do not understand why there is EXT_LANG_BT_ERROR. GDB has exceptions so
> it does not need error return values. Error return values can be used in cases
> when it is more simple for the code (As current implementation of GDB
> exceptions is needlessly complicated to use) - when callee detects a problem on
> its own. But if caller has to TRY_CATCH exceptions from futher callees and
> convert them into EXT_LANG_BT_ERROR so that its caller can check for
> EXT_LANG_BT_ERROR... Why hasn't the caller just do TRY_CATCH on its own?
> I haven't tried to remove EXT_LANG_BT_ERROR at all myself in this patchset,
> though. That would be larger work.
>
> As I said the patch is not complete but I believe the direction of this patch
> is correct and futher fixes can extend it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] framefilter quit: PR cli/17716
2015-02-10 17:40 ` [PATCH 0/6] framefilter quit: PR cli/17716 Pedro Alves
@ 2015-02-10 19:19 ` Phil Muldoon
0 siblings, 0 replies; 15+ messages in thread
From: Phil Muldoon @ 2015-02-10 19:19 UTC (permalink / raw)
To: Pedro Alves, Jan Kratochvil, gdb-patches
On 10/02/15 17:40, Pedro Alves wrote:
> Hi Jan,
>
> I read this, and it looks pretty good to me.
>
> Phil, any comments?
>
>
No I think it is great. I looked at it today, and just have not gotten around to writing email.
Thanks Jan for doing this.
Cheers
Phil
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 1/6] framefilter quit: Obvious whitespacing fixes
2015-02-07 14:45 ` [PATCH 1/6] framefilter quit: Obvious whitespacing fixes Jan Kratochvil
@ 2015-02-11 13:37 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:37 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:08 +0100, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * python/py-framefilter.c (py_print_frame): Whitespacing fixes.
8d4a54e2fb7f44c20ff3ddf42ff67db6bd08bdab master
25a0672ba8b7f4badc682eaf08a1e342b496b483 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 2/6] framefilter quit: Code cleanup: Reindentation
2015-02-07 14:45 ` [PATCH 2/6] framefilter quit: Code cleanup: Reindentation Jan Kratochvil
@ 2015-02-11 13:39 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:16 +0100, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * python/py-framefilter.c (py_print_frame): Put conditional code paths
> with goto first, indent the former else codepath left. Put variable
> 'elided' to a new inner block.
34019068f0082676b31926c7ec84dba0cfb2aba5 master
a9189a245363137825ce02a23202df45b04db179 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos
2015-02-07 14:45 ` [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos Jan Kratochvil
@ 2015-02-11 13:43 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:43 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:24 +0100, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * python/py-framefilter.c (py_print_frame): Substitute goto error.
> Remove the error label.
800eb1cebe736f6867d13e5df40a2c463a4b23ad master
46d020960241fcafd7d0ab483746a206829620ea 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 4/6] framefilter quit: Make it exception safe
2015-02-07 14:45 ` [PATCH 4/6] framefilter quit: Make it exception safe Jan Kratochvil
@ 2015-02-11 13:45 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:32 +0100, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * python/py-framefilter.c (py_print_frame): Mention RETURN_QUIT in
> function comment. Wrap all function that can throw in cleanups.
> (gdbpy_apply_frame_filter): Wrap all function that can throw in
> cleanups.
b99bf4e352f8590ccee3fbe3b4b031efdfcccdab master
5dea9fe2285fee5d53691856750a0c8960fca04d 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR
2015-02-07 14:45 ` [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR Jan Kratochvil
@ 2015-02-11 13:52 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:41 +0100, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * python/py-framefilter.c (py_print_single_arg, enumerate_locals)
> (py_print_frame): Use RETURN_MASK_ERROR.
e1fcd5757be08c23c5e72595d3cc4f5736fa7cda master
cf3f71d273c607ecd1e912ce9fe8121da320c6a9 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [commit+7.9] [PATCH 6/6] framefilter quit: New test
2015-02-07 14:45 ` [PATCH 6/6] framefilter quit: New test Jan Kratochvil
@ 2015-02-11 13:56 ` Jan Kratochvil
0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2015-02-11 13:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Phil Muldoon
On Sat, 07 Feb 2015 15:45:51 +0100, Jan Kratochvil wrote:
> gdb/testsuite/ChangeLog
> 2015-02-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.python/py-framefilter.exp (pagination quit - *): New tests.
63cc30e93a0a77a734ddf2f8ccf6e3b032248aea master
211e7f3c26b39159df2dedd8198148cec60307f9 7.9
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-02-11 13:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-07 14:45 [PATCH 0/6] framefilter quit: PR cli/17716 Jan Kratochvil
2015-02-07 14:45 ` [PATCH 5/6] framefilter quit: Use RETURN_MASK_ERROR Jan Kratochvil
2015-02-11 13:52 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 6/6] framefilter quit: New test Jan Kratochvil
2015-02-11 13:56 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 4/6] framefilter quit: Make it exception safe Jan Kratochvil
2015-02-11 13:45 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 1/6] framefilter quit: Obvious whitespacing fixes Jan Kratochvil
2015-02-11 13:37 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 3/6] framefilter quit: Code cleanup: Avoid gotos Jan Kratochvil
2015-02-11 13:43 ` [commit+7.9] " Jan Kratochvil
2015-02-07 14:45 ` [PATCH 2/6] framefilter quit: Code cleanup: Reindentation Jan Kratochvil
2015-02-11 13:39 ` [commit+7.9] " Jan Kratochvil
2015-02-10 17:40 ` [PATCH 0/6] framefilter quit: PR cli/17716 Pedro Alves
2015-02-10 19:19 ` Phil Muldoon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).