Re: testcase for "absolute source" patch

[Michael Chastain <mec dot gnu at mindspring dot com>]

Okay, here's a bunch of feedback on superficial stuff.
I haven't gotten to the substantive level of the test yet,
but it looks very useful.

  # Please email any bugs, comments, and/or additions to this file to:
  # bug-gdb@prep.ai.mit.edu

    This e-mail address is obsolete, so drop this.

  # XXX: We assume that build == host.

    Not good.  You have to at least try to work in a build != host
    environment.

    gdb_compile already downloads the srcfile from build to host.
    You can do "remote_download host" for more files, and
    "remote_exec host" to make directories and stuff.

  Style notes:

    Put a comment before each proc.  It can be as short as one line.

    Set the indentation level to 4 spaces.
    But tabs are still always eight spaces.
      (shiftwidth=4 tabstob=8 in vi)
      (i don't know the emacs settings)

    Check for errors on every system call, including cd.
    See lib/gdb.exp proc gdb_get_line_number for one way to do this;

      if { [ catch { cd ... } message ] } then {
	perror "$message"
	return -1
      }

    You don't need as many quotes and braces as you've been using.
    This works just fine:

      if { [gdb_compile $src $bin executable \

    It works even if $src or $bin have magic characters or spaces
    in them.  Unlike shell syntax, Tcl figures out the word boundaries
    first and then expands $src .  So $src always expands to exactly
    one word.

    If you look at gdb_get_line_number again, you will see that I
    did not know that either when I wrote it!

    Please don't name your variable "tmp".  Conceptually it's not
    even temporary; its value is the permanent directory that you
    keep coming back to.  How about "old_pwd" or "dir_current"
    or "oldpwd" or just about anything but "tmp".

  system

    Don't use system; use "remote_exec build ..." to run on the
    build machine or "remote_exec host ..." to run on the host
    machine.  You usually want "remote_exec host ...".

  echo

    Change this so that openp.c is a real file.  This makes it
    easier to find things.

    Also "main(){}" is not a legal Ansi C program.  Change it to "int
    main(){return 0;}" at least.  You can arrange the spacing any way
    you want -- it's explicitly encouraged to have a diversity of
    program formatting in the test programs.  But make it Ansi C.  Then
    you can drop the "-w" flag in gdb_compile.  Then I won't have
    problems later with hpux ansi c or ibm aix xlc.

    It's cool to have a one-line C program.  It's a trivial program
    so it does not need a copyright notice or license.

  Don't bother removing testtmpdir at the end unless it's really
  large, and it's nowhere near that large.  If somebody is
  investigating what happened with your test, they will want this
  directory to exist.