This is the mail archive of the gdb-patches@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]

Re: [patch] nto target update


Aleksandar Ristovski wrote:

> Nto target has been maintained in our private branch and hasn't been
> synchronized with mainstream for a while.
> As a result, nto target does not compile. This patch brings it up-to date.

Thank you very much for working on getting nto support in GDB mainline
up to date.  Unfortunately, this patch introduces a number of coding
style violations that I'd ask you to fix before this can be applied.

Also, the patch introduces a number of functions and macros that are
apparently not used anywhere, neither in the patch nor in the existing
nto code.  Is this something to support some other out-of-mainline code?
It would be better if we only add functions as they are actually needed
in mainline.

> 2007-11-07  Aleksandar Ristovski <aristovski@qnx.com>
> 
> 	Update for nto target.
> 
> 	* i386-nto-tdep.c: Update.
> 	* nto-procfs.c: Update.
> 	* nto-tdep.c: Update.
> 	* nto-tdep.h: Update.

Please provide a complete ChangeLog entry, listing every change
you made to those files.  See section 6.8 in 
http://www.gnu.org/prep/standards/standards.html
on how those logs should look like.


Some remarks on coding style and other issues:

-  sp = extract_unsigned_integer (buf, 4);
+  CORE_ADDR ptrctx;
 
-  return sp + I386_NTO_SIGCONTEXT_OFFSET;
+  /* we store __ucontext_t addr in EDI register */
+  ptrctx = frame_unwind_register_unsigned (next_frame, 
+  				I386_EDI_REGNUM);
+  ptrctx += 24 /* context pointer is at this offset */;

Comments should be full sentences starting with a capital letter and
ending with a full stop (followed by two spaces):

  /* We store the __ucontext_t address in the EDI register.  */


+      int sigwaitres;
       ofunc = (void (*)()) signal (SIGINT, nto_interrupt);
-      sigwaitinfo (&set, &info);
+      sigwaitres = sigwaitinfo (&set, &info);
+      if (sigwaitres == -1)
+        {
+	  internal_error (__FILE__, __LINE__ - 3, "sigwaitres failed with errno: %d\n", errno);
+        }

Is this really an internal_error, or more something along the lines
of perror_with_name?  If it is an internal error, I don't think
you should use __LINE__ - 3 here ...

+  pvoid = (void*)&run.fault;
Space between "void" and "*".  
+
+  psigset = (sigset_t *) pvoid;

Why is this double cast necessary in the first place?


+#include "gdb_assert.h"
+#include "gdbcmd.h"

Whenever you add include files to a file, please update the dependencies
in the Makefile.in.


+#define link_map 	so_map

This doesn't appear to be used anywhere.

@@ -154,7 +155,12 @@ nto_find_and_open_solib (char *solib, un
 	  if (ret >= 0)
 	    *temp_pathname = gdb_realpath (arch_path);
 	  else
-	    **temp_pathname = '\0';
+	    {
+	      if (*temp_pathname)
+	        **temp_pathname = '\0';
+	      else
+	        *temp_pathname = "";
+	    }

Should this not just always set *temp_pathname to NULL?


+#if defined (__MINGW32__)
+#define PATH_SEP ";"
+#else
+#define PATH_SEP ":"
+#endif

You should use the existing DIRNAME_SEPARATOR.

+  sprintf (buf, "set solib-search-path %s/%s" PATH_SEP "%s/%s", arch_path, "lib", arch_path, "usr/lib");

Lines should not exceed 80 characters in length.  Also, it would be better
to avoid using sprintf on fixed-size buffers.  Use xstrprintf instead.

+struct link_map_offsets *
+nto_generic_svr4_fetch_link_map_offsets (void)

Unused.

+#ifndef offsetof
+#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER)
+#endif
+#define fieldsize(TYPE, MEMBER) (sizeof (((TYPE *)0)->MEMBER))

Unused.

+  if (NULL == buf)
+    {
+      return 0;
+    }

GDB coding style would prefer not using braces for a single statement.

+  return extract_typed_address (so->lm_info->lm + lmo->l_addr_offset,
+               builtin_type_void_data_ptr);

Indentation looks odd.  Please use tabs to correspond to 8 spaces.

-  sec->addr = nto_truncate_ptr (sec->addr + LM_ADDR (so) - vaddr);
-  sec->endaddr = nto_truncate_ptr (sec->endaddr + LM_ADDR (so) - vaddr);
+  sec->addr = nto_truncate_ptr (sec->addr + LM_ADDR_FROM_LINK_MAP (so) - vaddr);
+  sec->endaddr = nto_truncate_ptr (sec->endaddr + LM_ADDR_FROM_LINK_MAP (so) - vaddr);

Lines too long.

+char *
+nto_target_extra_thread_info (struct thread_info *ti)
+{
+  if (ti && ti->private && ti->private->name[0])
+    return ti->private->name;
+  return "";
+}
+
+#ifndef __QNXNTO__
+
+#define NTO_SIGHUP      1   /* hangup */
+#define NTO_SIGINT      2   /* interrupt */
+#define NTO_SIGQUIT     3   /* quit */
+#define NTO_SIGILL      4   /* illegal instruction (not reset when caught) */
+#define NTO_SIGTRAP     5   /* trace trap (not reset when caught) */
+#define NTO_SIGIOT      6   /* IOT instruction */
+#define NTO_SIGABRT     6   /* used by abort */
+#define NTO_SIGEMT      7   /* EMT instruction */
+#define NTO_SIGDEADLK   7   /* Mutex deadlock */
+#define NTO_SIGFPE      8   /* floating point exception */
+#define NTO_SIGKILL     9   /* kill (cannot be caught or ignored) */
+#define NTO_SIGBUS      10  /* bus error */
+#define NTO_SIGSEGV     11  /* segmentation violation */
+#define NTO_SIGSYS      12  /* bad argument to system call */
+#define NTO_SIGPIPE     13  /* write on pipe with no reader */
+#define NTO_SIGALRM     14  /* real-time alarm clock */
+#define NTO_SIGTERM     15  /* software termination signal from kill */
+#define NTO_SIGUSR1     16  /* user defined signal 1 */
+#define NTO_SIGUSR2     17  /* user defined signal 2 */
+#define NTO_SIGCHLD     18  /* death of child */
+#define NTO_SIGPWR      19  /* power-fail restart */
+#define NTO_SIGWINCH    20  /* window change */
+#define NTO_SIGURG      21  /* urgent condition on I/O channel */
+#define NTO_SIGPOLL     22  /* System V name for NTO_SIGIO */
+#define NTO_SIGIO       NTO_SIGPOLL
+#define NTO_SIGSTOP     23  /* sendable stop signal not from tty */
+#define NTO_SIGTSTP     24  /* stop signal from tty */
+#define NTO_SIGCONT     25  /* continue a stopped process */
+#define NTO_SIGTTIN     26  /* attempted background tty read */
+#define NTO_SIGTTOU     27  /* attempted background tty write */
+#define NTO_SIGVTALRM   28  /* virtual timer expired */
+#define NTO_SIGPROF     29  /* profileing timer expired */
+#define NTO_SIGXCPU     30  /* exceded cpu limit */
+#define NTO_SIGXFSZ     31  /* exceded file size limit */
+
+static struct
+  {
+    int nto_sig;
+    enum target_signal gdb_sig;
+  }
+sig_map[] =
+{
+  {1, TARGET_SIGNAL_HUP},
+  {2, TARGET_SIGNAL_INT},
+  {3, TARGET_SIGNAL_QUIT},
+  {4, TARGET_SIGNAL_ILL},
+  {5, TARGET_SIGNAL_TRAP},
+  {6, TARGET_SIGNAL_ABRT},
+  {7, TARGET_SIGNAL_EMT},
+  {8, TARGET_SIGNAL_FPE},
+  {9, TARGET_SIGNAL_KILL},
+  {10, TARGET_SIGNAL_BUS},
+  {11, TARGET_SIGNAL_SEGV},
+  {12, TARGET_SIGNAL_SYS},
+  {13, TARGET_SIGNAL_PIPE},
+  {14, TARGET_SIGNAL_ALRM},
+  {15, TARGET_SIGNAL_TERM},
+  {16, TARGET_SIGNAL_USR1},
+  {17, TARGET_SIGNAL_USR2},
+  {18, TARGET_SIGNAL_CHLD},
+  {19, TARGET_SIGNAL_PWR},
+  {20, TARGET_SIGNAL_WINCH},
+  {21, TARGET_SIGNAL_URG},
+  {22, TARGET_SIGNAL_POLL},
+  {23, TARGET_SIGNAL_STOP},
+  {24, TARGET_SIGNAL_TSTP},
+  {25, TARGET_SIGNAL_CONT},
+  {26, TARGET_SIGNAL_TTIN},
+  {27, TARGET_SIGNAL_TTOU},
+  {28, TARGET_SIGNAL_VTALRM},
+  {29, TARGET_SIGNAL_PROF},
+  {30, TARGET_SIGNAL_XCPU},
+  {31, TARGET_SIGNAL_XFSZ}
+};
+#endif // ndef __QNXNTO__
+
+/* Convert nto signal to gdb signal.  */
+enum target_signal
+target_signal_from_nto(int sig)
+{
+#ifndef __QNXNTO__
+  switch(sig)
+  {
+    case 0: return 0; break;
+    case NTO_SIGHUP: return TARGET_SIGNAL_HUP; break;
+    case NTO_SIGINT: return TARGET_SIGNAL_INT; break;
+    case NTO_SIGQUIT: return TARGET_SIGNAL_QUIT; break;
+    case NTO_SIGILL: return TARGET_SIGNAL_ILL; break;
+    case NTO_SIGTRAP: return TARGET_SIGNAL_TRAP; break;
+    case NTO_SIGABRT: return TARGET_SIGNAL_ABRT; break;
+    case NTO_SIGEMT: return TARGET_SIGNAL_EMT; break;
+    case NTO_SIGFPE: return TARGET_SIGNAL_FPE; break;
+    case NTO_SIGKILL: return TARGET_SIGNAL_KILL; break;
+    case NTO_SIGBUS: return TARGET_SIGNAL_BUS; break;
+    case NTO_SIGSEGV: return TARGET_SIGNAL_SEGV; break;
+    case NTO_SIGSYS: return TARGET_SIGNAL_SYS; break;
+    case NTO_SIGPIPE: return TARGET_SIGNAL_PIPE; break;
+    case NTO_SIGALRM: return TARGET_SIGNAL_ALRM; break;
+    case NTO_SIGTERM: return TARGET_SIGNAL_TERM; break;
+    case NTO_SIGUSR1: return TARGET_SIGNAL_USR1; break;
+    case NTO_SIGUSR2: return TARGET_SIGNAL_USR2; break;
+    case NTO_SIGCHLD: return TARGET_SIGNAL_CHLD; break;
+    case NTO_SIGPWR: return TARGET_SIGNAL_PWR; break;
+    case NTO_SIGWINCH: return TARGET_SIGNAL_WINCH; break;
+    case NTO_SIGURG: return TARGET_SIGNAL_URG; break;
+    case NTO_SIGPOLL: return TARGET_SIGNAL_POLL; break;
+    case NTO_SIGSTOP: return TARGET_SIGNAL_STOP; break;
+    case NTO_SIGTSTP: return TARGET_SIGNAL_TSTP; break;
+    case NTO_SIGCONT: return TARGET_SIGNAL_CONT; break;
+    case NTO_SIGTTIN: return TARGET_SIGNAL_TTIN; break;
+    case NTO_SIGTTOU: return TARGET_SIGNAL_TTOU; break;
+    case NTO_SIGVTALRM: return TARGET_SIGNAL_VTALRM; break;
+    case NTO_SIGPROF: return TARGET_SIGNAL_PROF; break;
+    case NTO_SIGXCPU: return TARGET_SIGNAL_XCPU; break;
+    case NTO_SIGXFSZ: return TARGET_SIGNAL_XFSZ; break;
+    default: break;
+  }
+#endif /* __QNXNTO__ */
+  return target_signal_from_host(sig);
+}
+
+
+/* Convert gdb signal to nto signal.  */
+int
+target_signal_to_nto(enum target_signal sig)
+{
+#ifndef __QNXNTO__
+  switch(sig)
+  {
+    case 0: return 0; break;
+    case TARGET_SIGNAL_HUP: return NTO_SIGHUP; break;
+    case TARGET_SIGNAL_INT: return NTO_SIGINT; break;
+    case TARGET_SIGNAL_QUIT: return NTO_SIGQUIT; break;
+    case TARGET_SIGNAL_ILL: return NTO_SIGILL; break;
+    case TARGET_SIGNAL_TRAP: return NTO_SIGTRAP; break;
+    case TARGET_SIGNAL_ABRT: return NTO_SIGABRT; break;
+    case TARGET_SIGNAL_EMT: return NTO_SIGEMT; break;
+    case TARGET_SIGNAL_FPE: return NTO_SIGFPE; break;
+    case TARGET_SIGNAL_KILL: return NTO_SIGKILL; break;
+    case TARGET_SIGNAL_BUS: return NTO_SIGBUS; break;
+    case TARGET_SIGNAL_SEGV: return NTO_SIGSEGV; break;
+    case TARGET_SIGNAL_SYS: return NTO_SIGSYS; break;
+    case TARGET_SIGNAL_PIPE: return NTO_SIGPIPE; break;
+    case TARGET_SIGNAL_ALRM: return NTO_SIGALRM; break;
+    case TARGET_SIGNAL_TERM: return NTO_SIGTERM; break;
+    case TARGET_SIGNAL_USR1: return NTO_SIGUSR1; break;
+    case TARGET_SIGNAL_USR2: return NTO_SIGUSR2; break;
+    case TARGET_SIGNAL_CHLD: return NTO_SIGCHLD; break;
+    case TARGET_SIGNAL_PWR: return NTO_SIGPWR; break;
+    case TARGET_SIGNAL_WINCH: return NTO_SIGWINCH; break;
+    case TARGET_SIGNAL_URG: return NTO_SIGURG; break;
+    case TARGET_SIGNAL_IO: return NTO_SIGIO; break;
+    case TARGET_SIGNAL_POLL: return NTO_SIGPOLL; break;
+    case TARGET_SIGNAL_STOP: return NTO_SIGSTOP; break;
+    case TARGET_SIGNAL_TSTP: return NTO_SIGTSTP; break;
+    case TARGET_SIGNAL_CONT: return NTO_SIGCONT; break;
+    case TARGET_SIGNAL_TTIN: return NTO_SIGTTIN; break;
+    case TARGET_SIGNAL_TTOU: return NTO_SIGTTOU; break;
+    case TARGET_SIGNAL_VTALRM: return NTO_SIGVTALRM; break;
+    case TARGET_SIGNAL_PROF: return NTO_SIGPROF; break;
+    case TARGET_SIGNAL_XCPU: return NTO_SIGXCPU; break;
+    case TARGET_SIGNAL_XFSZ: return NTO_SIGXFSZ; break;
+    default: break;
+  }
+#endif /* __QNXNTO__ */
+  return target_signal_to_host(sig);
+}
+

Unless I'm missing something, all this seems unused.

+static void
+show_nto_debug (struct ui_file *file, int from_tty,
+                struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("QNX NTO debug level is %d.\n"), nto_internal_debugging);
Line too long.

+}
+
+static int 
+nto_print_tidinfo_callback (struct thread_info *tp, void *data)
+{
+  printf_filtered("%c%d\t%d\t%d\n", ptid_equal (tp->ptid, inferior_ptid) ? '*' : ' ', tp->private->tid, tp->private->state, tp->private->flags );

Line much too long.  Also, please use a space before the '(' in a function call.

+  return 0;
+}
+
+static void 
+nto_info_tidinfo_command (char *args, int from_tty)
+{
+  nto_trace (0) ("%s (args=%s, from_tty=%d)\n", __func__, args, from_tty);
+
+  target_find_new_threads ();
+  printf_filtered("Threads for pid %d (%s)\nTid:\tState:\tFlags:\n", ptid_get_pid (inferior_ptid), get_exec_file (0));

Likewise.

 
+#define nto_trace(level) \
+  if ((nto_internal_debugging & 0xFF) <= level) {} else \
+    printf_unfiltered

I'm not sure this kind of tracing macro belongs in 
mainline; but I won't object to it either if you feel
you need it ...

+/* register supply helper macros*/
+#define NTO_ALL_REGS (-1)
+#define RAW_SUPPLY_IF_NEEDED(regcache, whichreg, dataptr) \
+  {if (!(NTO_ALL_REGS == regno || regno == (whichreg))) {} \
+    else regcache_raw_supply (regcache, whichreg, dataptr); }

Apparently unused. 


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  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]