Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Kevin Buettner <kevinb@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFC] remote-mips.c: Make sure that each function has an introductory comment
Date: Tue, 09 Mar 2010 17:50:00 -0000	[thread overview]
Message-ID: <83lje12xma.fsf@gnu.org> (raw)
In-Reply-To: <20100308151822.2adfab51@redhat.com>

> Date: Mon, 8 Mar 2010 15:18:22 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> After Joel noticed that a couple of functions in one of my recent
> patch submissions for remote-mips.c were missing comments, I noticed
> that there were more than a few functions in remote-mips.c that are
> missing introductory comments.  Many of the new comments in the patch
> below describe "obvious" functions, but some of them weren't so
> obvious.  Since there were a few non-obvious ones, I decided to ask
> for comments prior to committing.  So...

Thanks for working on this.

> Comments?

Just a few, from the POV of a more-or-less naive reader:

> +/* Add the checksum specified by *VALUE to the buffer, the last location
> +   of which, so far, is specified by *BUF.  At the time of this call,
> +   the current size of the buffer is RECSIZE.

The usual ambiguity of *size parameters is whether or not it includes
the trailing null character.  Many times, the only way to know is to
read the code.

>                                                    Return the total size
> +   of the buffer after adding the checksum escape, the checksum itself,
> +   and the trailing newline.

Again, does this include the null?

> +                            INAMOUNT is the size of INBUF.

Size of the buffer or length of the text in it?

> +   *RECSIZE will be written with the total size of the output buffer
> +   prior to returning.

With or without the null?

> +/* Look for the string specified by STRING sent from the target board
> +   during a download operation.  If the string in question is not seen,
> +   output an error message, clean up, and return 0.

What does it mean "clean up" in this case?  If it's important to know
about it, perhaps we should tell more; if it's not important, why
mention it?


  parent reply	other threads:[~2010-03-09 17:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-08 22:18 Kevin Buettner
2010-03-09  5:25 ` Joel Brobecker
2010-03-09 17:50 ` Eli Zaretskii [this message]
2010-03-10  5:37   ` Kevin Buettner
2010-03-10 17:08     ` Eli Zaretskii
2010-03-11  3:48     ` Kevin Buettner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83lje12xma.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=kevinb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox