[patch] nto target: Code cleanup

[patch] nto target: Code cleanup

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

Hello,

This patch removes i386_nto_target as unnecessary indirection.

Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems

ChangeLog:

	* i386-nto-tdep.c (i386_nto_target): Remove definition.
	(init_i386nto_ops): Use macros to set fields to global
	current_nto_target directly.
	(i386nto_init_abi): Remove unused nto_set_target call.
	* nto-tdep.h (nto_set_target): Remove unused declaration.
	* nto-tdep.c (nto_set_target): Remove unused function.

[-- Attachment #2: i386-nto-tdep.c-removei386-nto-tdep-20090611.diff --]
[-- Type: text/x-patch, Size: 3174 bytes --]

Index: gdb/i386-nto-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nto-tdep.c,v
retrieving revision 1.33
diff -u -p -r1.33 i386-nto-tdep.c
--- gdb/i386-nto-tdep.c	11 Jun 2009 17:12:11 -0000	1.33
+++ gdb/i386-nto-tdep.c	11 Jun 2009 19:12:33 -0000
@@ -34,9 +34,6 @@
 #include "solib.h"
 #include "solib-svr4.h"
 
-/* Target vector for QNX NTO x86.  */
-static struct nto_target_ops i386_nto_target;
-
 #ifndef X86_CPU_FXSR
 #define X86_CPU_FXSR (1L << 12)
 #endif
@@ -310,14 +307,14 @@ i386nto_sigcontext_addr (struct frame_in
 static void
 init_i386nto_ops (void)
 {
-  i386_nto_target.regset_id = i386nto_regset_id;
-  i386_nto_target.supply_gregset = i386nto_supply_gregset;
-  i386_nto_target.supply_fpregset = i386nto_supply_fpregset;
-  i386_nto_target.supply_altregset = nto_dummy_supply_regset;
-  i386_nto_target.supply_regset = i386nto_supply_regset;
-  i386_nto_target.register_area = i386nto_register_area;
-  i386_nto_target.regset_fill = i386nto_regset_fill;
-  i386_nto_target.fetch_link_map_offsets =
+  nto_regset_id = i386nto_regset_id;
+  nto_supply_gregset = i386nto_supply_gregset;
+  nto_supply_fpregset = i386nto_supply_fpregset;
+  nto_supply_altregset = nto_dummy_supply_regset;
+  nto_supply_regset = i386nto_supply_regset;
+  nto_register_area = i386nto_register_area;
+  nto_regset_fill = i386nto_regset_fill;
+  nto_fetch_link_map_offsets =
     svr4_ilp32_fetch_link_map_offsets;
 }
 
@@ -371,8 +368,6 @@ i386nto_init_abi (struct gdbarch_info in
         = nto_in_dynsym_resolve_code;
     }
   set_solib_ops (gdbarch, &nto_svr4_so_ops);
-
-  nto_set_target (&i386_nto_target);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
Index: gdb/nto-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/nto-tdep.h,v
retrieving revision 1.14
diff -u -p -r1.14 nto-tdep.h
--- gdb/nto-tdep.h	7 Jun 2009 17:58:24 -0000	1.14
+++ gdb/nto-tdep.h	11 Jun 2009 19:12:33 -0000
@@ -142,8 +142,6 @@ typedef struct _debug_regs
 
 void nto_init_solib_absolute_prefix (void);
 
-void nto_set_target(struct nto_target_ops *);
-
 char **nto_parse_redirection (char *start_argv[], const char **in,
 			      const char **out, const char **err);
 
Index: gdb/nto-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/nto-tdep.c,v
retrieving revision 1.32
diff -u -p -r1.32 nto-tdep.c
--- gdb/nto-tdep.c	22 May 2009 23:49:13 -0000	1.32
+++ gdb/nto-tdep.c	11 Jun 2009 19:12:33 -0000
@@ -67,19 +67,6 @@ nto_target (void)
 #endif
 }
 
-void
-nto_set_target (struct nto_target_ops *targ)
-{
-  nto_regset_id = targ->regset_id;
-  nto_supply_gregset = targ->supply_gregset;
-  nto_supply_fpregset = targ->supply_fpregset;
-  nto_supply_altregset = targ->supply_altregset;
-  nto_supply_regset = targ->supply_regset;
-  nto_register_area = targ->register_area;
-  nto_regset_fill = targ->regset_fill;
-  nto_fetch_link_map_offsets = targ->fetch_link_map_offsets;
-}
-
 /* Take a string such as i386, rs6000, etc. and map it onto CPUTYPE_X86,
    CPUTYPE_PPC, etc. as defined in nto-share/dsmsgs.h.  */
 int

Re: [patch] nto target: Code cleanup

[Joel Brobecker]

> 	* i386-nto-tdep.c (i386_nto_target): Remove definition.
> 	(init_i386nto_ops): Use macros to set fields to global
> 	current_nto_target directly.
> 	(i386nto_init_abi): Remove unused nto_set_target call.
> 	* nto-tdep.h (nto_set_target): Remove unused declaration.
> 	* nto-tdep.c (nto_set_target): Remove unused function.

Thanks for extracting this part out. This is OK.

I'm wondering how the other targets do this sort of thing,
but my brain seem to be refusing to function at the moment...

-- 
Joel

Re: [patch] nto target: Code cleanup

[Aleksandar Ristovski]

Joel Brobecker wrote:
>> 	* i386-nto-tdep.c (i386_nto_target): Remove definition.
>> 	(init_i386nto_ops): Use macros to set fields to global
>> 	current_nto_target directly.
>> 	(i386nto_init_abi): Remove unused nto_set_target call.
>> 	* nto-tdep.h (nto_set_target): Remove unused declaration.
>> 	* nto-tdep.c (nto_set_target): Remove unused function.
> 
> Thanks for extracting this part out. This is OK.
> 
> I'm wondering how the other targets do this sort of thing,
> but my brain seem to be refusing to function at the moment...
> 
Thanks for quick review!

Committed.

Primary motivation for this gymnastics in nto was that we 
actually support 5 different cpu platforms and code in 
nto-tdep.c is "generic", that is it calls particular nto 
target's (that is cpu specific stuff) from there.

My motivation for cleanup is to remove things that don't 
seem to do what is expected (like in this example, by 
removing this indirection I haven't changed behaviour at all 
since the "nto_set_target" simply copies just set functions 
from i386_nto_target into current_nto_target.

The whole thing will undergo further cleanups, but right now 
I want to bring it as close to what I have as possible.


-- 
Aleksandar Ristovski
QNX Software Systems

Re: [patch] nto target: Code cleanup

[Mark Kettenis]

> From: Aleksandar Ristovski <aristovski@qnx.com>
> Date:  Thu, 11 Jun 2009 15:15:56 -0400
> 
> Hello,
> 
> This patch removes i386_nto_target as unnecessary indirection.
> 
> Thanks,
> 
> -- 
> Aleksandar Ristovski
> QNX Software Systems
> 
> ChangeLog:
> 
> 	* i386-nto-tdep.c (i386_nto_target): Remove definition.
> 	(init_i386nto_ops): Use macros to set fields to global
> 	current_nto_target directly.
> 	(i386nto_init_abi): Remove unused nto_set_target call.
> 	* nto-tdep.h (nto_set_target): Remove unused declaration.
> 	* nto-tdep.c (nto_set_target): Remove unused function.

Not sure this is a step in the right direction.  This being in a
-tdep.c file pretty much makes it impossible to build a multi-arch GDB
that supports multiple NTO targets.

Re: [patch] nto target: Code cleanup

[Aleksandar Ristovski]

Mark Kettenis wrote:
>> From: Aleksandar Ristovski <aristovski@qnx.com>
>> Date:  Thu, 11 Jun 2009 15:15:56 -0400
>>
>> Hello,
>>
>> This patch removes i386_nto_target as unnecessary indirection.
>>
>> Thanks,
>>
>> -- 
>> Aleksandar Ristovski
>> QNX Software Systems
>>
>> ChangeLog:
>>
>> 	* i386-nto-tdep.c (i386_nto_target): Remove definition.
>> 	(init_i386nto_ops): Use macros to set fields to global
>> 	current_nto_target directly.
>> 	(i386nto_init_abi): Remove unused nto_set_target call.
>> 	* nto-tdep.h (nto_set_target): Remove unused declaration.
>> 	* nto-tdep.c (nto_set_target): Remove unused function.
> 
> Not sure this is a step in the right direction.  This being in a
> -tdep.c file pretty much makes it impossible to build a multi-arch GDB
> that supports multiple NTO targets.
> 

The patch functionally doesn't change anything. If you look 
at nto-tdep.h there is current_nto_target extern which gets 
set either by macros I used in my patch or indirectly by 
nto_set_target.

The patch removes the unnecessary indirection since it 
doesn't do what one might expect.

Note that this approach works for multiarch but will 
definitely not work once we enable gdb for multiple targets 
(of different architectures) at the same time, and my patch 
doesn't change this situation.

Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems

Re: [patch] nto target: Code cleanup

[Aleksandar Ristovski]

Aleksandar Ristovski wrote:
> Mark Kettenis wrote:
>>
>> Not sure this is a step in the right direction.  This being in a
>> -tdep.c file pretty much makes it impossible to build a multi-arch GDB
>> that supports multiple NTO targets.
>>
> 
> The patch functionally doesn't change anything. If you look at 
> nto-tdep.h there is current_nto_target extern which gets set either by 
> macros I used in my patch or indirectly by nto_set_target.
> 
> The patch removes the unnecessary indirection since it doesn't do what 
> one might expect.
> 
> Note that this approach works for multiarch but will definitely not work 
> once we enable gdb for multiple targets (of different architectures) at 
> the same time, and my patch doesn't change this situation.
> 

Mark do you agree? Are you ok with the patch?


Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems