This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Stop using errno values around target_xfer interfaces and memory errors.
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 09 Oct 2013 17:28:26 +0100
- Subject: Re: [PATCH] Stop using errno values around target_xfer interfaces and memory errors.
- Authentication-results: sourceware.org; auth=none
- References: <20130823163640 dot 17252 dot 39659 dot stgit at brno dot lan>
On 08/23/2013 05:36 PM, Pedro Alves wrote:
> target_read_memory & friends build on top of target_read (thus on top
> of the target_xfer machinery), but turn all errors to EIO, an errno
> value. I think we'd better convert all these to return a
> target_xfer_error too, like target_xfer_partial in a previous patch.
> The patch starts by doing that.
>
> (The patch does not add a enum target_xfer_error value for '0'/no
> error, and likewise does not change the return type of several of
> these functions to enum target_xfer_error, because different functions
> return '0' with different semantics.)
>
> I audited the tree for memory_error calls, EIO checks, places where
> GDB hardcodes 'errno = EIO', and also for strerror calls. What I
> found is that nowadays there's really no need to handle random errno
> values, other than the EIOs gdb itself hardcodes. No doubt errno
> values would appear in common code back in the day when
> target_xfer_memory was the main interface to access memory, but
> nowadays, any errno value that deprecated interface could return is
> just absorved by default_xfer_partial:
>
> else if (xfered == 0 && errno == 0)
> /* "deprecated_xfer_memory" uses 0, cross checked against
> ERRNO as one indication of an error. */
> return 0;
> else
> return -1;
>
> There are two places in the code that check for EIO and print "out of
> bounds", and defer to strerror for other errors. That's
> c-lang.c:c_get_string, and valprint.c.:val_print_string. AFAICT, the
> strerror branch can never be reached nowadays, as the only error
> possible to get at those points is EIO, given that it's GDB itself
> that set that errno value (in target_read_memory, etc.).
>
> breakpoint.c:insert_bp_location always prints the error val as if an
> errno, returned by target_insert_breakpoint, with strerr. Now the
> error here is either always EIO for mem-break.c targets (again
> hardcoded by the target_read_memory/target_write_memory functions), so
> this always prints "Input/output error" or similar (depending on
> host), or, for remote targets (and probably others), this gem:
>
> Error accessing memory address 0x80200400: Unknown error -1.
>
> This patch makes these 3 places print the exact same error
> memory_error prints. This changes output, but I think this is better,
> for making memory error output consistent with other commands, and, it
> means we have a central place to tweak for memory errors.
>
> E.g., this changes:
>
> Cannot insert breakpoint 1.
> Error accessing memory address 0x5fc660: Input/output error.
>
> to:
>
> Cannot insert breakpoint 1.
> Cannot access memory at address 0x5fc660
>
> Which I find pretty much acceptable.
>
> Surprisingly, only py-prettyprint.exp had a regression, for needing an
> adjustment. I also grepped the testsuite for the old errors, and
> found no other hits.
>
> Now that errno values aren't used anywhere in any of these memory
> access related routines, I made memory_error itself take a
> target_xfer_error instead of an errno. The new
> target_xfer_memory_error function added recently is no longer
> necessary, and is thus removed.
>
> Tested on x86_64 Fedora 17, native and gdbserver.
>
> gdb/
> 2013-08-23 Pedro Alves <palves@redhat.com>
>
> * breakpoint.c (insert_bp_location): Use memory_error_message to
> build the memory error string.
> * c-lang.c: Include "gdbcore.h".
> (c_get_string): Use memory_error to throw error.
> (target_xfer_memory_error): Delete.
> (memory_error_message): New, factored out from
> target_xfer_memory_error.
> (memory_error): Change parameter type to target_xfer_error.
> Rewrite.
> (read_memory): Use memory_error instead of
> target_xfer_memory_error.
> * gdbcore.h: Include "target.h".
> (memory_error): Change parameter type to target_xfer_error.
> (memory_error_message): Declare function.
> * target.c (target_read_memory, target_read_stack)
> (target_write_memory, target_write_raw_memory): Return
> TARGET_XFER_E_IO on error. Adjust comments.
> (get_target_memory): Pass TARGET_XFER_E_IO to memory_error,
> instead of EIO.
> * target.h (target_read, target_insert_breakpoint)
> (target_remove_breakpoint): Adjust comments.
> * valprint.c (partial_memory_read): Rename parameter, and adjust
> comment.
> (val_print_string): Use memory_error_message to build the memory
> error string.
>
> gdb/testsuite/
> 2013-08-23 Pedro Alves <palves@redhat.com>
>
> * gdb.python/py-prettyprint.exp (run_lang_tests): Adjust expected
> output.
I'm checking this in.
--
Pedro Alves