[c++,05/12] guile: Constify gdbscm_with_guile return value
Commit Message
On 26 October 2015 at 12:21, Doug Evans <xdje42@gmail.com> wrote:
> The function comment for gdbscm_with_guile says:
>
> /* A wrapper around scm_with_guile that prints backtraces and exceptions
> according to "set guile print-stack".
> The result if NULL if no exception occurred, otherwise it is a statically
> allocated error message (caller must *not* free). */
>
> If we're going to return an error message,
> why make it a void * and not a char * (const as appropriate)?
>
> The lower level guile API uses a void * because it doesn't specify what
> the result is. But in this use of it we do specify what the result is.
It totally makes sense. Here is the updated patch (probably garbled
by gmail, sorry about that):
From 2defdf7de52146ea7508d0dda2f0c59e7fdd42fe Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon dot marchi at polymtl dot ca>
Date: Sun, 25 Oct 2015 23:46:37 -0400
Subject: [PATCH] guile: Change return value of gdbscm_with_guile for const
char *
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The documentation of gdbscm_with_guile says that it returns a statically
allocated string (IOW, a const char *). We can reflect that in its
return value type, and get rid of C++ build errors.
Initially fixes:
/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function
‘void* gdbscm_disasm_read_memory_worker(void*)’:
/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error:
invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
return "seek error";
gdb/ChangeLog:
* guile/guile-internal.h (gdbscm_with_guile): Change return
types to const char *.
* guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
(struct c_data) <func>: Likewise.
(struct c_data) <result>: Change type to const char *.
(scscm_eval_scheme_string): Change return type to
const char *.
(scscm_source_scheme_script): Likewise.
(gdbscm_safe_eval_string): Change type of result variable to
const char * and remove cast.
(gdbscm_safe_source_script): Likewise.
* guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
Change return type to const char *.
(gdbscm_disasm_read_memory): Change type of status to
const char *.
---
gdb/guile/guile-internal.h | 2 +-
gdb/guile/scm-disasm.c | 4 ++--
gdb/guile/scm-safe-call.c | 20 ++++++++++----------
3 files changed, 13 insertions(+), 13 deletions(-)
const char *filename = (const char *) data;
@@ -439,7 +439,7 @@ gdbscm_safe_source_script (const char *filename)
by default. This function is invoked by the "source" GDB command which
already has its own path search support. */
char *abs_filename = NULL;
- void *result;
+ const char *result;
if (!IS_ABSOLUTE_PATH (filename))
{
@@ -452,7 +452,7 @@ gdbscm_safe_source_script (const char *filename)
xfree (abs_filename);
if (result != NULL)
- return xstrdup ((char *) result);
+ return xstrdup (result);
return NULL;
}
Comments
Simon Marchi <simon.marchi@polymtl.ca> writes:
> On 26 October 2015 at 12:21, Doug Evans <xdje42@gmail.com> wrote:
>> The function comment for gdbscm_with_guile says:
>>
>> /* A wrapper around scm_with_guile that prints backtraces and exceptions
>> according to "set guile print-stack".
>> The result if NULL if no exception occurred, otherwise it is a statically
>> allocated error message (caller must *not* free). */
>>
>> If we're going to return an error message,
>> why make it a void * and not a char * (const as appropriate)?
>>
>> The lower level guile API uses a void * because it doesn't specify what
>> the result is. But in this use of it we do specify what the result is.
>
> It totally makes sense. Here is the updated patch (probably garbled
> by gmail, sorry about that):
>
>
> From 2defdf7de52146ea7508d0dda2f0c59e7fdd42fe Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon dot marchi at polymtl dot ca>
> Date: Sun, 25 Oct 2015 23:46:37 -0400
> Subject: [PATCH] guile: Change return value of gdbscm_with_guile for const
> char *
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The documentation of gdbscm_with_guile says that it returns a statically
> allocated string (IOW, a const char *). We can reflect that in its
> return value type, and get rid of C++ build errors.
>
> Initially fixes:
>
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function
> ‘void* gdbscm_disasm_read_memory_worker(void*)’:
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error:
> invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
> return "seek error";
>
> gdb/ChangeLog:
>
> * guile/guile-internal.h (gdbscm_with_guile): Change return
> types to const char *.
> * guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
> (struct c_data) <func>: Likewise.
> (struct c_data) <result>: Change type to const char *.
> (scscm_eval_scheme_string): Change return type to
> const char *.
> (scscm_source_scheme_script): Likewise.
> (gdbscm_safe_eval_string): Change type of result variable to
> const char * and remove cast.
> (gdbscm_safe_source_script): Likewise.
> * guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
> Change return type to const char *.
> (gdbscm_disasm_read_memory): Change type of status to
> const char *.
LGTM.
Thanks!
On 27 October 2015 at 11:54, Doug Evans <xdje42@gmail.com> wrote:
> LGTM.
> Thanks!
Pushed, thanks!
@@ -384,7 +384,7 @@ extern void gdbscm_memory_error (const char *subr,
const char *msg, SCM args)
/* scm-safe-call.c */
-extern void *gdbscm_with_guile (void *(*func) (void *), void *data);
+extern const char *gdbscm_with_guile (const char *(*func) (void *),
void *data);
extern SCM gdbscm_call_guile (SCM (*func) (void *), void *data,
excp_matcher_func *ok_excps);
@@ -76,7 +76,7 @@ dascm_make_insn (CORE_ADDR pc, const char *assembly,
int insn_len)
Scheme port. Called via gdbscm_call_guile.
The result is a statically allocated error message or NULL if success. */
-static void *
+static const char *
gdbscm_disasm_read_memory_worker (void *datap)
{
struct gdbscm_disasm_read_data *data
@@ -109,7 +109,7 @@ gdbscm_disasm_read_memory (bfd_vma memaddr,
bfd_byte *myaddr,
struct disassemble_info *dinfo)
{
struct gdbscm_disasm_read_data data;
- void *status;
+ const char *status;
data.memaddr = memaddr;
data.myaddr = myaddr;
@@ -28,10 +28,10 @@
struct c_data
{
- void *(*func) (void *);
+ const char *(*func) (void *);
void *data;
/* An error message or NULL for success. */
- void *result;
+ const char *result;
};
/* Struct to marshall args through gdbscm_with_catch. */
@@ -167,8 +167,8 @@ gdbscm_with_catch (void *data)
The result if NULL if no exception occurred, otherwise it is a statically
allocated error message (caller must *not* free). */
-void *
-gdbscm_with_guile (void *(*func) (void *), void *data)
+const char *
+gdbscm_with_guile (const char *(*func) (void *), void *data)
{
struct c_data c_data;
struct with_catch_data catch_data;
@@ -369,7 +369,7 @@ struct eval_scheme_string_data
/* Wrapper to eval a C string in the Guile interpreter.
This is passed to gdbscm_with_guile. */
-static void *
+static const char *
scscm_eval_scheme_string (void *datap)
{
struct eval_scheme_string_data *data
@@ -398,12 +398,12 @@ char *
gdbscm_safe_eval_string (const char *string, int display_result)
{
struct eval_scheme_string_data data = { string, display_result };
- void *result;
+ const char *result;
result = gdbscm_with_guile (scscm_eval_scheme_string, (void *) &data);
if (result != NULL)
- return xstrdup ((char *) result);
+ return xstrdup (result);
return NULL;
}
@@ -411,7 +411,7 @@ gdbscm_safe_eval_string (const char *string, int
display_result)
/* Helper function for gdbscm_safe_source_scheme_script. */
-static void *
+static const char *
scscm_source_scheme_script (void *data)
{