Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Eli Zaretskii" <eliz@gnu.org>
To: Mark Kettenis <kettenis@gnu.org>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [COMMIT] Improve coding standard in dbxread.c
Date: Sat, 20 Nov 2004 17:22:00 -0000	[thread overview]
Message-ID: <01c4cf25$Blat.v2.2.2$44ec6860@zahav.net.il> (raw)
In-Reply-To: <200411201239.iAKCd9O5061792@elgar.sibelius.xs4all.nl> (message from Mark Kettenis on Sat, 20 Nov 2004 13:39:09 +0100 (CET))

> Date: Sat, 20 Nov 2004 13:39:09 +0100 (CET)
> From: Mark Kettenis <kettenis@gnu.org>
> 
> Only a tiny fraction of the code, but it's a step into the right
> direction.

I think changes like these are wrong:

> -    case N_STSYM:		/* Static symbol in data seg */
> -    case N_LCSYM:		/* Static symbol in BSS seg */
> -    case N_ROSYM:		/* Static symbol in Read-only data seg */
> +    case N_STSYM:		/* Static symbol in data segment.  */
> +    case N_LCSYM:		/* Static symbol in BSS segment.  */
> +    case N_ROSYM:		/* Static symbol in read-only data segment.  */

The text in these comments does not constitute a full sentence, and so
adding a period at the end is not the right change.  The right way to
fix this, IMHO, is to lower-case the first letter of the comment, like
this:

    case N_STSYM:		/* static symbol in data segment */

There are numerous other instances of similar changes, and IMHO they
all are wrong.  This one looks particularly incorrect after the
change:

> +    case N_NOMAP:		/* No map?  (Ultrix).  */

Some of the other changes simply reformat comments to break the line
at a different column.  Do we have a canonical column number for that,
and if we do, what is its value?


  reply	other threads:[~2004-11-20 17:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-11-20 12:39 Mark Kettenis
2004-11-20 17:22 ` Eli Zaretskii [this message]
2004-11-20 17:41   ` Mark Kettenis
2004-11-20 18:00     ` Eli Zaretskii

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='01c4cf25$Blat.v2.2.2$44ec6860@zahav.net.il' \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=kettenis@gnu.org \
    /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