On 09/03/2013 10:38 AM, Thomas Schwinge wrote: > Hi! > > For context, Yue Lu is a student participating in this year's Google > Summer of Code program, to port gdbserver to GNU Hurd, and is both a GDB > as well as a GNU Hurd newbie. (So, be gentle.) ;-) > > On Tue, 3 Sep 2013 16:00:32 +0800, Yue Lu <hacklu.newb...@gmail.com> wrote: >> This is my patch to port gdbserver to GNU/Hurd. Most of code are >> copied from [gdb]/gdb/gnu-nat.c. > > ... and elsewhere. Our strategy was to first get this into a basic > functional state: > >> Now the gdbserver on GNU/Hurd can set breakpoint and check memory or >> register(but without float-register support). > > So, this initial port just posted is a great milestone! Especially so > given your previous lack of experience with both GDB and the Hurd -- both > of which are not always easy to grasp.
Indeed! > > There are lots of things to be polished (Yue, don't worry -- this > entirely was to be expected), such as GDB coding standard, and changes > that are unrelated to your port, which all has to be cleared out before I > can commit this initial port to GDB upstream. > > There is missing functionality, but we decided this can be enhanced piece > by piece once the initial port is accepted. > > There is the issue of code sharing between GDB proper and gdbserver, a > strategy for which has been briefly discussed in > <http://news.gmane.org/find-root.php?message_id=%3CCAB8fV%3Djzv_rPHP3-HQVBA-pCNZNat6PNbh%2BOJEU7tZgQdKX3%2Bw%40mail.gmail.com%3E>. That has been further discussed recently: https://sourceware.org/ml/gdb-patches/2013-07/msg00788.html and we ended up with putting native target files under the new gdb/nat/ directory, rather than in gdb/common/: https://sourceware.org/ml/gdb-patches/2013-08/msg00618.html > Likewise for code sharing between the new Hurd gdbserver port and the > existing x86 Linux kernel port. Again I suggest this to be done *after* > the initial port is accepted: this will turn into a nice (and easily > reviewable) series of cleanup patches à la: remove from GDB proper > gdb/gnu-nat.c:[function] and from gdbserver > gdb/gdbserver/gnu-low.c:[function], and add > gdb/common/gnu-low.c:[function], and so on. Likewise for build > infrastructure that can be shared. > > Does this strategy generally make sense to you GDB maintainers? I've been thinking about this this morning, after seeing these patches. For new gdbserver ports, this path just seems to swim further away from a full sharing approach, by adding lots duplication as first step, and actually making it hard to see what exactly needed to be changed/adapted for gdbserver use, and puts the tree in a state where any further changes for the GNU/Hurd target will need to be considered twice going further, exactly what we're fighting against with the existing ports. I think that strategy ultimately is slower to get at where we want to, and is actually more work than an alternative that does things the other way around. I checked out Yue's git branch, and diffed gdb/gnu-nat.c vs gdbserver/gnu-low.c, and gdb/i386gnu-nat.c vs gdbserver/gnu-i386-low.c. I didn't diff the rest of the files, as I assume they'll probably have even less divergence. There are several spurious formatting differences, and some reordering of functions, but for the most port, the code is mostly identical. There's some expected necessary adjustment to GDBserver's interfaces, but it turns out it's not that much. We've been converging gdb's and gdbserver's interfaces throughout the years, and it now shows. So my idea would be, instead of adding the new files under gdbserver, to remove the spurious differences (formatting, reordering, etc.) that were introduced in the gdbserver copies of the files, eliminating the unnecessary divergence, and then fold back the resulting differences into the original gdb/gnu-nat.c etc. files, guarded by #ifdef GDBSERVER. Some cleanups might have been identified and done in the gdbserver files, and it might make sense to do those as preparatory work, in the original files. This should result in smaller patches, and will actually avoid the need for most of the polishing Thomas mentioned, and as consequence review burden -- reviewing the new gnu-low.c etc., for GNU conventions, formatting, or even appropriate use of the Hurd's debug APIs etc., is just unnecessary that way, by design, and we'll be able to focus on the bits that are the real new code -- the glue between the target and gdb, and the target and gdbserver. The current state of the work isn't wasted at all! And I don't think following this direction is that much work. I'd do this my literally moving gdbserver/gnu-low.c on top of gdb/gnu-nat.c (etc.), and use git diff to guide me through, in identifying what would need to be restored, and guarded with #if[n]def GDBSERVER. #ifdef GDBSERVER is how we've adapting shared code under gdb/common/ and gdb/nat/ to work on both programs. Medium/long term, core gdb and core gdbserver target interface differences should converge further, and the #ifdefs disappear, but for now that's a necessary evil. It's fine to leave bits of functionality disabled on gdbserver, wrapped in #ifndef GDBSERVER. After that initial work is committed, we can then easily progress the gdbserver port by just looking for those #ifdefs. It's fine with me to leave the existing native files under gdb/ while doing this; it's probably easier that way. Moving them under gdb/nat/ can be left for a cleanup step further down the road. Could we try that approach? -- Pedro Alves
