This is the mail archive of the gdb@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

libgdb functions return value inconsistency


Hello,

while tracking down an MI regression between GDB 6.3 and 6.5 I noticed
an apparent inconsistency in the libgdb return code handling.

First, the regression: when setting a breakpoint via MI into a file
that isn't found, MI used to return an error, but now it reports
success:

GDB 6.3
(gdb) 
151-break-insert shared.c:5
&"No source file named shared.c.\n"
No source file named shared.c.
151^error,msg="No source file named shared.c."
(gdb) 

GDB 6.5
(gdb) 
122-break-insert simple_spu.c:6
&"No source file named simple_spu.c.\n"
No source file named simple_spu.c.
122^done
(gdb) 


In GDB 6.5 the error is detected in symtab_from_filename (linespec.c),
which does a throw_error.  This is caught in gdb_breakpoint (breakpoint.c),
which has does:

enum gdb_rc
gdb_breakpoint (char *address, char *condition,
                int hardwareflag, int tempflag,
                int thread, int ignore_count,
                char **error_message)
{
[snip]
  return catch_exceptions_with_msg (uiout, do_captured_breakpoint, &args,
                                    error_message, RETURN_MASK_ALL);
}

The catch_exceptions_with_msg call returns RETURN_ERROR (i.e. -1).

However, the return value of gdb_breakpoint is interpreted as "enum gdb_rc"
and checked by the caller mi_cmd_break_insert (mi/mi-cmd-break.c) against
GDB_RC_FAIL (i.e. 0).  As -1 != 0, the caller assumes everything went well.

In GDB 6.3, the gdb_breakpoint function was using catch_errors, which
returns 0 on failure -- this matched the GDB_RC_FAIL code (which appears
to be intentional, according to a comment in gdb.h).


However, looking over all four gdb_... functions defined to return an
enum gdb_rc, you'll notice that as of today they *all* use
catch_exceptions_with_msg to determine their return value.

On the other hand, there is one function defined in wrapper.h to return
enum gdb_rc, and that function makes sure to only return GDB_RC_FAIL
or GDB_RC_OK.


Now, looking at the call sites, we have this code in mi/mi-main.c:

enum mi_cmd_result
mi_cmd_thread_select (char *command, char **argv, int argc)
{
  enum gdb_rc rc;
[snip]
    rc = gdb_thread_select (uiout, argv[0], &mi_error_message);

  /* RC is enum gdb_rc if it is successful (>=0)
     enum return_reason if not (<0). */
  if ((int) rc < 0 && (enum return_reason) rc == RETURN_ERROR)
    return MI_CMD_ERROR;
  else if ((int) rc >= 0 && rc == GDB_RC_FAIL)
    return MI_CMD_ERROR;
  else
    return MI_CMD_DONE;
}

But then:

enum mi_cmd_result
mi_cmd_thread_list_ids (char *command, char **argv, int argc)
{
  enum gdb_rc rc = MI_CMD_DONE;
[snip]
    rc = gdb_list_thread_ids (uiout, &mi_error_message);

  if (rc == GDB_RC_FAIL)
    return MI_CMD_ERROR;
  else
    return MI_CMD_DONE;
}


This is all trivial to fix one way or the other.  But the first question is,
what *should* the interface be?  Should those functions return RETURN_ERROR
or GDB_RC_FAIL (or either?) on error?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]