* Re: patch ping: QNX Neutrino remote debug support
2003-10-10 14:23 patch ping: QNX Neutrino remote debug support Kris Warkentin
@ 2003-10-11 19:03 ` Andrew Cagney
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cagney @ 2003-10-11 19:03 UTC (permalink / raw)
To: Kris Warkentin; +Cc: Gdb-Patches@Sources.Redhat.Com
I obviously can't review the protocol implementation. So ...
Two comming changes to keep in mind:
- having multiple live target stacks is on the horizon, that will meen
that most statics will need to be consulated into a struct
- async is similar (assuming it can be re-started). you'll want to
think about the consequences.
anyway, some more general comments ...
> #include <fcntl.h>
> #include <signal.h>
"defs.h" is _always_ included first. I also noticed that one of the
other headers was pulling in <stdlib.h>? Is it needed.
Is all of the following needed? Look at how remote-io.c handled this
problem.
> #ifdef __CYGWIN__
> #include <sys/cygwin.h>
> #endif
>
> #ifndef EOK
> #define EOK 0
> #endif
>
> #define QNX_READ_MODE 0x0
> #define QNX_WRITE_MODE 0x301
> #define QNX_WRITE_PERMS 0x1ff
>
> #ifdef __CYGWIN__
> #define HOST_READ_MODE O_RDONLY|O_BINARY
> #define HOST_WRITE_MODE O_WRONLY|O_CREAT|O_TRUNC|O_BINARY
> #else
> #define HOST_READ_MODE O_RDONLY
> #define HOST_WRITE_MODE O_WRONLY|O_CREAT|O_TRUNC
> #endif
>
> #ifndef O_BINARY
> #define O_BINARY 0
> #endif
> int (*target_created_hook) (pid_t);
GDB's trying to switch to observers, but first, is this even used? Or
is another hook/observer already available?
> #define SWAP64( val ) ( \
> #define SWAP32( val ) ( (((val) >> 24) & 0x000000ff) \
> #define SWAP16( val ) ( (((val) >> 8) & 0xff) | (((val) << 8) & 0xff00) )
> nto_swap16 (int val)
> nto_swap32 (long int val)
> nto_swap64 (long long int val)
These all shouldn't be needed. Instead extract_[un]signed_integer
should be able to do the job (I'm guessing that this part of the
protocol is target byteordered).
Use LONGEST if you need a large integer.
> #define SEND_NAK serial_write(nto_desc,nak_packet,sizeof(nak_packet))
> #define SEND_CH_RESET serial_write(nto_desc,ch_reset_packet,sizeof(ch_reset_packet))
> #define SEND_CH_DEBUG serial_write(nto_desc,ch_debug_packet,sizeof(ch_debug_packet))
> #define SEND_CH_TEXT serial_write(nto_desc,ch_text_packet,sizeof(ch_text_packet))
Shouldn't these be functions?
> printf_unfiltered ("[escape]");
debug info should go to gdb_stdlog.
> nto_desc = serial_open (name);
Ya!
[about here I maxed out]
> static void
> nto_add_commands ()
> {
> struct cmd_list_element *c;
>
> c =
> add_com ("upload", class_obscure, upload_command,
> "Send a file to the target (upload {local} [{remote}])");
> c->completer = filename_completer;
> add_com ("download", class_obscure, download_command,
> "Get a file from the target (download {remote} [{local}])");
> }
>
> static void
> nto_remove_commands ()
> {
> extern struct cmd_list_element *cmdlist;
>
> delete_cmd ("upload", &cmdlist);
> delete_cmd ("download", &cmdlist);
> }
Can you start a discussion about these commands (I guess on gdb@)? (For
the moment, leave them disabled?).
The revamping of "to_query" opens the potential for implementing this in
a clean generic way.
> void
> _initialize_nto ()
Can this be moved to the end of the file?
> add_show_from_set (add_set_cmd ("qnxtimeout", no_class,
> var_integer, (char *) &nto_timeout,
> "Set timeout value for remote read.\n",
> &setlist), &showlist);
>
> add_show_from_set (add_set_cmd ("qnxinheritenv", no_class,
> var_boolean, (char *) &nto_inherit_env,
> "Set where remote process inherits env from pdebug, or has it set by gdb\n",
> &setlist), &showlist);
>
> add_show_from_set (add_set_cmd ("qnxremotecwd", class_support, var_string,
> (char *) &nto_remote_cwd,
> "Set the working directory for the remote process",
> &setlist), &showlist);
>
> add_setshow_cmd ("nto-remote-file", class_files, var_string,
> &nto_remote_file, "Set the remote file to be executed \
> when a user issues the 'run'\ncommand to a QNX \
> Neutrino remote target.\n", "Show the file to be executed on the remote\n\
> QNX Neutrino target.\n", NULL, NULL, &setlist, &showlist);
>
> add_setshow_cmd ("upload-sets-exec", class_files, var_integer,
> &upload_sets_exec,
> "If set, upload will set nto_remote_file.\n",
> "Show the flag for upload to set nto_remote_file.\n", NULL,
> NULL, &setlist, &showlist);
See "remote.c" for how to set things up so that "set remote-nto ...", or
"set nto ...?" works. You'll, separatly, need doco.
For the "set qnx*", suggest, locally, investigating GDB's mechanism for
deprecating commands.
BTW, "nto_remove_file" isn't meaningful.
> add_info ("pidlist", nto_pidlist, "pidlist");
> add_info ("meminfo", nto_meminfo, "memory information");
Can you explain these? "info threads" vs "pidlist". "maint info nto
memory" or "info memory" vs "meminfo"? As with "download", disable them
for now.
Is the below needed? I don't think it belongs in remote-nto as even a
native will need to do this transformation.
> #ifndef __QNXNTO__
>
> #define QNXSIGHUP 1 /* Hangup. */
> #define QNXSIGINT 2 /* Interrupt. */
> #define QNXSIGQUIT 3 /* Quit. */
> #define QNXSIGILL 4 /* Illegal instruction (not reset when caught). */
> #define QNXSIGTRAP 5 /* Trace trap (not reset when caught). */
> #define QNXSIGIOT 6 /* IOT instruction. */
> #define QNXSIGABRT 6 /* Used by abort. */
Andrew
^ permalink raw reply [flat|nested] 2+ messages in thread