From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12038 invoked by alias); 11 Oct 2003 19:03:35 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 12020 invoked from network); 11 Oct 2003 19:03:30 -0000 Received: from unknown (HELO localhost.redhat.com) (65.49.0.121) by sources.redhat.com with SMTP; 11 Oct 2003 19:03:30 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id E54462B89; Sat, 11 Oct 2003 15:03:10 -0400 (EDT) Message-ID: <3F8853EE.3080702@redhat.com> Date: Sat, 11 Oct 2003 19:03:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kris Warkentin Cc: "Gdb-Patches@Sources.Redhat.Com" Subject: Re: patch ping: QNX Neutrino remote debug support References: <02bb01c38f3a$446b6570$0202040a@catdog> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-10/txt/msg00410.txt.bz2 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 > #include "defs.h" is _always_ included first. I also noticed that one of the other headers was pulling in ? Is it needed. Is all of the following needed? Look at how remote-io.c handled this problem. > #ifdef __CYGWIN__ > #include > #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