public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [doc RFA] Clean up coding standards, add python coding standards.
@ 2010-10-19 15:53 Doug Evans
  2010-10-19 18:45 ` Eli Zaretskii
  0 siblings, 1 reply; 3+ messages in thread
From: Doug Evans @ 2010-10-19 15:53 UTC (permalink / raw)
  To: gdb-patches

Hi.

This patch does a couple of things.

I think it is wrong that a discussion of cleanups appears in the same
chapter as coding standards.  Ditto for a few other subsections
in the existing Coding chapter.

This patch creates a new Coding Standards chapter to put such things in,
and I've added a section on Python coding standards.

I'm not happy with the name "Miscellaneous Coding Guidelines" but it
works well enough for me.  Feel free to suggest an alternatives.

Ok to check in?

2010-10-19  Doug Evans  <dje@google.com>

	* gdbint.texinfo (Miscellaneous Coding Guidelines): Renamed from
	Coding.  All references updated.
	(Coding Standards): New chapter.  Move the coding standard related
	subsections here.  Add section on Python coding standards.

Index: gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.326
diff -u -p -r1.326 gdbint.texinfo
--- gdbint.texinfo	22 Jun 2010 05:03:19 -0000	1.326
+++ gdbint.texinfo	19 Oct 2010 15:48:33 -0000
@@ -83,7 +83,8 @@ as the mechanisms that adapt @value{GDBN
 * Target Vector Definition::
 * Native Debugging::
 * Support Libraries::
-* Coding::
+* Miscellaneous Coding Guidelines::
+* Coding Standards::
 * Porting GDB::
 * Versions and Branches::
 * Start of New Year Procedure::
@@ -1322,9 +1323,9 @@ be signaled.
 
 @deftypefun {struct cleanup *} make_cleanup_ui_out_tuple_begin_end (struct ui_out *@var{uiout}, const char *@var{id})
 This function first opens the tuple and then establishes a cleanup
-(@pxref{Coding, Cleanups}) to close the tuple.  It provides a convenient
-and correct implementation of the non-portable@footnote{The function
-cast is not portable ISO C.} code sequence:
+(@pxref{Miscellaneous Coding Guidelines, Cleanups}) to close the tuple.
+It provides a convenient and correct implementation of the
+non-portable@footnote{The function cast is not portable ISO C.} code sequence:
 @smallexample
 struct cleanup *old_cleanup;
 ui_out_tuple_begin (uiout, "...");
@@ -1349,7 +1350,8 @@ be signaled.
 
 @deftypefun {struct cleanup *} make_cleanup_ui_out_list_begin_end (struct ui_out *@var{uiout}, const char *@var{id})
 Similar to @code{make_cleanup_ui_out_tuple_begin_end}, this function
-opens a list and then establishes cleanup (@pxref{Coding, Cleanups})
+opens a list and then establishes cleanup
+(@pxref{Miscellaneous Coding Guidelines, Cleanups})
 that will close the list.
 @end deftypefun
 
@@ -5756,9 +5758,9 @@ Binary search the array.
 
 @section include
 
-@node Coding
+@node Miscellaneous Coding Guidelines
 
-@chapter Coding
+@chapter Miscellaneous Coding Guidelines
 
 This chapter covers topics that are lower-level than the major
 algorithms of @value{GDBN}.
@@ -5995,27 +5997,6 @@ finish by printing a newline, to flush t
 to unfiltered (@code{printf}) output.  Symbol reading routines that
 print warnings are a good example.
 
-@section @value{GDBN} Coding Standards
-@cindex coding standards
-
-@value{GDBN} follows the GNU coding standards, as described in
-@file{etc/standards.texi}.  This file is also available for anonymous
-FTP from GNU archive sites.  @value{GDBN} takes a strict interpretation
-of the standard; in general, when the GNU standard recommends a practice
-but does not require it, @value{GDBN} requires it.
-
-@value{GDBN} follows an additional set of coding standards specific to
-@value{GDBN}, as described in the following sections.
-
-
-@subsection ISO C
-
-@value{GDBN} assumes an ISO/IEC 9899:1990 (a.k.a.@: ISO C90) compliant
-compiler.
-
-@value{GDBN} does not assume an ISO C or POSIX compliant C library.
-
-
 @subsection Memory Management
 
 @value{GDBN} does not use the functions @code{malloc}, @code{realloc},
@@ -6109,6 +6090,186 @@ currently too noisy to enable with @samp
 
 @end table
 
+@subsection Internal Error Recovery
+
+During its execution, @value{GDBN} can encounter two types of errors.
+User errors and internal errors.  User errors include not only a user
+entering an incorrect command but also problems arising from corrupt
+object files and system errors when interacting with the target.
+Internal errors include situations where @value{GDBN} has detected, at
+run time, a corrupt or erroneous situation.
+
+When reporting an internal error, @value{GDBN} uses
+@code{internal_error} and @code{gdb_assert}.
+
+@value{GDBN} must not call @code{abort} or @code{assert}.
+
+@emph{Pragmatics: There is no @code{internal_warning} function.  Either
+the code detected a user error, recovered from it and issued a
+@code{warning} or the code failed to correctly recover from the user
+error and issued an @code{internal_error}.}
+
+@subsection Command Names
+
+GDB U/I commands are written @samp{foo-bar}, not @samp{foo_bar}.
+
+@subsection Clean Design and Portable Implementation
+
+@cindex design
+In addition to getting the syntax right, there's the little question of
+semantics.  Some things are done in certain ways in @value{GDBN} because long
+experience has shown that the more obvious ways caused various kinds of
+trouble.
+
+@cindex assumptions about targets
+You can't assume the byte order of anything that comes from a target
+(including @var{value}s, object files, and instructions).  Such things
+must be byte-swapped using @code{SWAP_TARGET_AND_HOST} in
+@value{GDBN}, or one of the swap routines defined in @file{bfd.h},
+such as @code{bfd_get_32}.
+
+You can't assume that you know what interface is being used to talk to
+the target system.  All references to the target must go through the
+current @code{target_ops} vector.
+
+You can't assume that the host and target machines are the same machine
+(except in the ``native'' support modules).  In particular, you can't
+assume that the target machine's header files will be available on the
+host machine.  Target code must bring along its own header files --
+written from scratch or explicitly donated by their owner, to avoid
+copyright problems.
+
+@cindex portability
+Insertion of new @code{#ifdef}'s will be frowned upon.  It's much better
+to write the code portably than to conditionalize it for various
+systems.
+
+@cindex system dependencies
+New @code{#ifdef}'s which test for specific compilers or manufacturers
+or operating systems are unacceptable.  All @code{#ifdef}'s should test
+for features.  The information about which configurations contain which
+features should be segregated into the configuration files.  Experience
+has proven far too often that a feature unique to one particular system
+often creeps into other systems; and that a conditional based on some
+predefined macro for your current system will become worthless over
+time, as new versions of your system come out that behave differently
+with regard to this feature.
+
+Adding code that handles specific architectures, operating systems,
+target interfaces, or hosts, is not acceptable in generic code.
+
+@cindex portable file name handling
+@cindex file names, portability
+One particularly notorious area where system dependencies tend to
+creep in is handling of file names.  The mainline @value{GDBN} code
+assumes Posix semantics of file names: absolute file names begin with
+a forward slash @file{/}, slashes are used to separate leading
+directories, case-sensitive file names.  These assumptions are not
+necessarily true on non-Posix systems such as MS-Windows.  To avoid
+system-dependent code where you need to take apart or construct a file
+name, use the following portable macros:
+
+@table @code
+@findex HAVE_DOS_BASED_FILE_SYSTEM
+@item HAVE_DOS_BASED_FILE_SYSTEM
+This preprocessing symbol is defined to a non-zero value on hosts
+whose filesystems belong to the MS-DOS/MS-Windows family.  Use this
+symbol to write conditional code which should only be compiled for
+such hosts.
+
+@findex IS_DIR_SEPARATOR
+@item IS_DIR_SEPARATOR (@var{c})
+Evaluates to a non-zero value if @var{c} is a directory separator
+character.  On Unix and GNU/Linux systems, only a slash @file{/} is
+such a character, but on Windows, both @file{/} and @file{\} will
+pass.
+
+@findex IS_ABSOLUTE_PATH
+@item IS_ABSOLUTE_PATH (@var{file})
+Evaluates to a non-zero value if @var{file} is an absolute file name.
+For Unix and GNU/Linux hosts, a name which begins with a slash
+@file{/} is absolute.  On DOS and Windows, @file{d:/foo} and
+@file{x:\bar} are also absolute file names.
+
+@findex FILENAME_CMP
+@item FILENAME_CMP (@var{f1}, @var{f2})
+Calls a function which compares file names @var{f1} and @var{f2} as
+appropriate for the underlying host filesystem.  For Posix systems,
+this simply calls @code{strcmp}; on case-insensitive filesystems it
+will call @code{strcasecmp} instead.
+
+@findex DIRNAME_SEPARATOR
+@item DIRNAME_SEPARATOR
+Evaluates to a character which separates directories in
+@code{PATH}-style lists, typically held in environment variables.
+This character is @samp{:} on Unix, @samp{;} on DOS and Windows.
+
+@findex SLASH_STRING
+@item SLASH_STRING
+This evaluates to a constant string you should use to produce an
+absolute filename from leading directories and the file's basename.
+@code{SLASH_STRING} is @code{"/"} on most systems, but might be
+@code{"\\"} for some Windows-based ports.
+@end table
+
+In addition to using these macros, be sure to use portable library
+functions whenever possible.  For example, to extract a directory or a
+basename part from a file name, use the @code{dirname} and
+@code{basename} library functions (available in @code{libiberty} for
+platforms which don't provide them), instead of searching for a slash
+with @code{strrchr}.
+
+Another way to generalize @value{GDBN} along a particular interface is with an
+attribute struct.  For example, @value{GDBN} has been generalized to handle
+multiple kinds of remote interfaces---not by @code{#ifdef}s everywhere, but
+by defining the @code{target_ops} structure and having a current target (as
+well as a stack of targets below it, for memory references).  Whenever
+something needs to be done that depends on which remote interface we are
+using, a flag in the current target_ops structure is tested (e.g.,
+@code{target_has_stack}), or a function is called through a pointer in the
+current target_ops structure.  In this way, when a new remote interface
+is added, only one module needs to be touched---the one that actually
+implements the new remote interface.  Other examples of
+attribute-structs are BFD access to multiple kinds of object file
+formats, or @value{GDBN}'s access to multiple source languages.
+
+Please avoid duplicating code.  For example, in @value{GDBN} 3.x all
+the code interfacing between @code{ptrace} and the rest of
+@value{GDBN} was duplicated in @file{*-dep.c}, and so changing
+something was very painful.  In @value{GDBN} 4.x, these have all been
+consolidated into @file{infptrace.c}.  @file{infptrace.c} can deal
+with variations between systems the same way any system-independent
+file would (hooks, @code{#if defined}, etc.), and machines which are
+radically different don't need to use @file{infptrace.c} at all.
+
+All debugging code must be controllable using the @samp{set debug
+@var{module}} command.  Do not use @code{printf} to print trace
+messages.  Use @code{fprintf_unfiltered(gdb_stdlog, ...}.  Do not use
+@code{#ifdef DEBUG}.
+
+@node Coding Standards
+
+@chapter Coding Standards
+@cindex coding standards
+
+@section @value{GDBN} C Coding Standards
+
+@value{GDBN} follows the GNU coding standards, as described in
+@file{etc/standards.texi}.  This file is also available for anonymous
+FTP from GNU archive sites.  @value{GDBN} takes a strict interpretation
+of the standard; in general, when the GNU standard recommends a practice
+but does not require it, @value{GDBN} requires it.
+
+@value{GDBN} follows an additional set of coding standards specific to
+@value{GDBN}, as described in the following sections.
+
+@subsection ISO C
+
+@value{GDBN} assumes an ISO/IEC 9899:1990 (a.k.a.@: ISO C90) compliant
+compiler.
+
+@value{GDBN} does not assume an ISO C or POSIX compliant C library.
+
 @subsection Formatting
 
 @cindex source code formatting
@@ -6208,7 +6369,6 @@ protected with parentheses.)
 Declarations like @samp{struct foo *} should be used in preference to
 declarations like @samp{typedef struct foo @{ @dots{} @} *foo_ptr}.
 
-
 @subsection Function Prototypes
 @cindex function prototypes
 
@@ -6225,30 +6385,6 @@ visible to random source files.
 Where a source file needs a forward declaration of a static function,
 that declaration must appear in a block near the top of the source file.
 
-
-@subsection Internal Error Recovery
-
-During its execution, @value{GDBN} can encounter two types of errors.
-User errors and internal errors.  User errors include not only a user
-entering an incorrect command but also problems arising from corrupt
-object files and system errors when interacting with the target.
-Internal errors include situations where @value{GDBN} has detected, at
-run time, a corrupt or erroneous situation.
-
-When reporting an internal error, @value{GDBN} uses
-@code{internal_error} and @code{gdb_assert}.
-
-@value{GDBN} must not call @code{abort} or @code{assert}.
-
-@emph{Pragmatics: There is no @code{internal_warning} function.  Either
-the code detected a user error, recovered from it and issued a
-@code{warning} or the code failed to correctly recover from the user
-error and issued an @code{internal_error}.}
-
-@subsection Command Names
-
-GDB U/I commands are written @samp{foo-bar}, not @samp{foo_bar}.
-
 @subsection File Names
 
 Any file used when building the core of @value{GDBN} must be in lower
@@ -6278,7 +6414,6 @@ programs, e.g.@: @value{GDBN} and @sc{gd
 
 For other files @samp{-} is used as the separator.
 
-
 @subsection Include Files
 
 A @file{.c} file should include @file{defs.h} first.
@@ -6313,141 +6448,31 @@ header body
 #endif
 @end smallexample
 
+@section @value{GDBN} Python Coding Standards
 
-@subsection Clean Design and Portable Implementation
-
-@cindex design
-In addition to getting the syntax right, there's the little question of
-semantics.  Some things are done in certain ways in @value{GDBN} because long
-experience has shown that the more obvious ways caused various kinds of
-trouble.
-
-@cindex assumptions about targets
-You can't assume the byte order of anything that comes from a target
-(including @var{value}s, object files, and instructions).  Such things
-must be byte-swapped using @code{SWAP_TARGET_AND_HOST} in
-@value{GDBN}, or one of the swap routines defined in @file{bfd.h},
-such as @code{bfd_get_32}.
-
-You can't assume that you know what interface is being used to talk to
-the target system.  All references to the target must go through the
-current @code{target_ops} vector.
-
-You can't assume that the host and target machines are the same machine
-(except in the ``native'' support modules).  In particular, you can't
-assume that the target machine's header files will be available on the
-host machine.  Target code must bring along its own header files --
-written from scratch or explicitly donated by their owner, to avoid
-copyright problems.
-
-@cindex portability
-Insertion of new @code{#ifdef}'s will be frowned upon.  It's much better
-to write the code portably than to conditionalize it for various
-systems.
-
-@cindex system dependencies
-New @code{#ifdef}'s which test for specific compilers or manufacturers
-or operating systems are unacceptable.  All @code{#ifdef}'s should test
-for features.  The information about which configurations contain which
-features should be segregated into the configuration files.  Experience
-has proven far too often that a feature unique to one particular system
-often creeps into other systems; and that a conditional based on some
-predefined macro for your current system will become worthless over
-time, as new versions of your system come out that behave differently
-with regard to this feature.
-
-Adding code that handles specific architectures, operating systems,
-target interfaces, or hosts, is not acceptable in generic code.
-
-@cindex portable file name handling
-@cindex file names, portability
-One particularly notorious area where system dependencies tend to
-creep in is handling of file names.  The mainline @value{GDBN} code
-assumes Posix semantics of file names: absolute file names begin with
-a forward slash @file{/}, slashes are used to separate leading
-directories, case-sensitive file names.  These assumptions are not
-necessarily true on non-Posix systems such as MS-Windows.  To avoid
-system-dependent code where you need to take apart or construct a file
-name, use the following portable macros:
-
-@table @code
-@findex HAVE_DOS_BASED_FILE_SYSTEM
-@item HAVE_DOS_BASED_FILE_SYSTEM
-This preprocessing symbol is defined to a non-zero value on hosts
-whose filesystems belong to the MS-DOS/MS-Windows family.  Use this
-symbol to write conditional code which should only be compiled for
-such hosts.
-
-@findex IS_DIR_SEPARATOR
-@item IS_DIR_SEPARATOR (@var{c})
-Evaluates to a non-zero value if @var{c} is a directory separator
-character.  On Unix and GNU/Linux systems, only a slash @file{/} is
-such a character, but on Windows, both @file{/} and @file{\} will
-pass.
-
-@findex IS_ABSOLUTE_PATH
-@item IS_ABSOLUTE_PATH (@var{file})
-Evaluates to a non-zero value if @var{file} is an absolute file name.
-For Unix and GNU/Linux hosts, a name which begins with a slash
-@file{/} is absolute.  On DOS and Windows, @file{d:/foo} and
-@file{x:\bar} are also absolute file names.
-
-@findex FILENAME_CMP
-@item FILENAME_CMP (@var{f1}, @var{f2})
-Calls a function which compares file names @var{f1} and @var{f2} as
-appropriate for the underlying host filesystem.  For Posix systems,
-this simply calls @code{strcmp}; on case-insensitive filesystems it
-will call @code{strcasecmp} instead.
+@value{GDBN} follows the published @code{Python} coding standards in
+@code{PEP008}.
+See @uref{http://www.python.org/dev/peps/pep-0008/, PEP008}.
 
-@findex DIRNAME_SEPARATOR
-@item DIRNAME_SEPARATOR
-Evaluates to a character which separates directories in
-@code{PATH}-style lists, typically held in environment variables.
-This character is @samp{:} on Unix, @samp{;} on DOS and Windows.
+In addition, the Google standards are also followed where they do not
+conflict with @code{PEP008}.
+See @uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
+Google Python Style Guide}.
 
-@findex SLASH_STRING
-@item SLASH_STRING
-This evaluates to a constant string you should use to produce an
-absolute filename from leading directories and the file's basename.
-@code{SLASH_STRING} is @code{"/"} on most systems, but might be
-@code{"\\"} for some Windows-based ports.
-@end table
+@subsection @value{GDBN}-specific exceptions
 
-In addition to using these macros, be sure to use portable library
-functions whenever possible.  For example, to extract a directory or a
-basename part from a file name, use the @code{dirname} and
-@code{basename} library functions (available in @code{libiberty} for
-platforms which don't provide them), instead of searching for a slash
-with @code{strrchr}.
+There are a few exceptions to the published standards.
+They exist mainly for consistency with the @code{C} standards.
 
-Another way to generalize @value{GDBN} along a particular interface is with an
-attribute struct.  For example, @value{GDBN} has been generalized to handle
-multiple kinds of remote interfaces---not by @code{#ifdef}s everywhere, but
-by defining the @code{target_ops} structure and having a current target (as
-well as a stack of targets below it, for memory references).  Whenever
-something needs to be done that depends on which remote interface we are
-using, a flag in the current target_ops structure is tested (e.g.,
-@code{target_has_stack}), or a function is called through a pointer in the
-current target_ops structure.  In this way, when a new remote interface
-is added, only one module needs to be touched---the one that actually
-implements the new remote interface.  Other examples of
-attribute-structs are BFD access to multiple kinds of object file
-formats, or @value{GDBN}'s access to multiple source languages.
+@c It is expected that there are a few more exceptions,
+@c so we use itemize here.
 
-Please avoid duplicating code.  For example, in @value{GDBN} 3.x all
-the code interfacing between @code{ptrace} and the rest of
-@value{GDBN} was duplicated in @file{*-dep.c}, and so changing
-something was very painful.  In @value{GDBN} 4.x, these have all been
-consolidated into @file{infptrace.c}.  @file{infptrace.c} can deal
-with variations between systems the same way any system-independent
-file would (hooks, @code{#if defined}, etc.), and machines which are
-radically different don't need to use @file{infptrace.c} at all.
+@itemize @bullet
 
-All debugging code must be controllable using the @samp{set debug
-@var{module}} command.  Do not use @code{printf} to print trace
-messages.  Use @code{fprintf_unfiltered(gdb_stdlog, ...}.  Do not use
-@code{#ifdef DEBUG}.
+@item
+Use @code{FIXME} instead of @code{TODO}.
 
+@end itemize
 
 @node Porting GDB
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [doc RFA] Clean up coding standards, add python coding standards.
  2010-10-19 15:53 [doc RFA] Clean up coding standards, add python coding standards Doug Evans
@ 2010-10-19 18:45 ` Eli Zaretskii
  2010-10-20 22:56   ` Doug Evans
  0 siblings, 1 reply; 3+ messages in thread
From: Eli Zaretskii @ 2010-10-19 18:45 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

> Date: Tue, 19 Oct 2010 08:53:10 -0700 (PDT)
> From: dje@google.com (Doug Evans)
> 
> I think it is wrong that a discussion of cleanups appears in the same
> chapter as coding standards.  Ditto for a few other subsections
> in the existing Coding chapter.
> 
> This patch creates a new Coding Standards chapter to put such things in,
> and I've added a section on Python coding standards.

Thanks.

> I'm not happy with the name "Miscellaneous Coding Guidelines" but it
> works well enough for me.  Feel free to suggest an alternatives.

"Misc Guidelines"?  In general, node names should be short, because
they are displayed on the mode line and header line of the Info
readers, where space is at premium.  (Section names, OTOH, can be
longer.)

> Ok to check in?

Could you tell me if you made any changes in the "Internal Error
Handling" part, or just moved it without any changes?  If you just
moved it, I won't need to review that part.

> -* Coding::
> +* Miscellaneous Coding Guidelines::
> +* Coding Standards::

I think "Coding Standards" should be before the new "Misc" section.

> +@value{GDBN} follows the published @code{Python} coding standards in
> +@code{PEP008}.
> +See @uref{http://www.python.org/dev/peps/pep-0008/, PEP008}.

This is suboptimal usage of @uref.  I suggest this instead:

  @value{GDBN} follows the published @code{Python} coding standards in
  @uref{http://www.python.org/dev/peps/pep-0008/, @code{PEP008}}.

> +In addition, the Google standards are also followed where they do not
> +conflict with @code{PEP008}.
> +See @uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
> +Google Python Style Guide}.

Same here:

  In addition, the guidelines in the
  @uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
  Google Python Style Guide} are also followed where they do not
  conflict with @code{PEP008}.

Okay with those changes (assuming that the "Internal Error Handling"
stuff was moved without any change).

Thanks for taking care of this.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [doc RFA] Clean up coding standards, add python coding standards.
  2010-10-19 18:45 ` Eli Zaretskii
@ 2010-10-20 22:56   ` Doug Evans
  0 siblings, 0 replies; 3+ messages in thread
From: Doug Evans @ 2010-10-20 22:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]

On Tue, Oct 19, 2010 at 11:44 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Tue, 19 Oct 2010 08:53:10 -0700 (PDT)
> > From: dje@google.com (Doug Evans)
> >
> > I think it is wrong that a discussion of cleanups appears in the same
> > chapter as coding standards.  Ditto for a few other subsections
> > in the existing Coding chapter.
> >
> > This patch creates a new Coding Standards chapter to put such things in,
> > and I've added a section on Python coding standards.
>
> Thanks.
>
> > I'm not happy with the name "Miscellaneous Coding Guidelines" but it
> > works well enough for me.  Feel free to suggest an alternatives.
>
> "Misc Guidelines"?  In general, node names should be short, because
> they are displayed on the mode line and header line of the Info
> readers, where space is at premium.  (Section names, OTOH, can be
> longer.)
>
> > Ok to check in?
>
> Could you tell me if you made any changes in the "Internal Error
> Handling" part, or just moved it without any changes?  If you just
> moved it, I won't need to review that part.
>
> > -* Coding::
> > +* Miscellaneous Coding Guidelines::
> > +* Coding Standards::
>
> I think "Coding Standards" should be before the new "Misc" section.
>
> > +@value{GDBN} follows the published @code{Python} coding standards in
> > +@code{PEP008}.
> > +See @uref{http://www.python.org/dev/peps/pep-0008/, PEP008}.
>
> This is suboptimal usage of @uref.  I suggest this instead:
>
>  @value{GDBN} follows the published @code{Python} coding standards in
>  @uref{http://www.python.org/dev/peps/pep-0008/, @code{PEP008}}.
>
> > +In addition, the Google standards are also followed where they do not
> > +conflict with @code{PEP008}.
> > +See @uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
> > +Google Python Style Guide}.
>
> Same here:
>
>  In addition, the guidelines in the
>  @uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
>  Google Python Style Guide} are also followed where they do not
>  conflict with @code{PEP008}.
>
> Okay with those changes (assuming that the "Internal Error Handling"
> stuff was moved without any change).
>
> Thanks for taking care of this.

Here is the patch I checked in.
Besides making the requested changes, I noticed that some sections in
"Misc Guidelines" were marked as subsections and fixed them too.

2010-10-20  Doug Evans  <dje@google.com>

        * gdbint.texinfo (Misc Guidelines): Renamed from Coding.
        All references updated.  Correct sections marked as subsections.
        (Coding Standards): New chapter.  Move the coding standard related
        subsections here.  Add section on Python coding standards.

[-- Attachment #2: gdb-101020-python-coding-2.patch.txt --]
[-- Type: text/plain, Size: 19411 bytes --]

2010-10-20  Doug Evans  <dje@google.com>

	* gdbint.texinfo (Misc Guidelines): Renamed from Coding.
	All references updated.  Correct sections marked as subsections.
	(Coding Standards): New chapter.  Move the coding standard related
	subsections here.  Add section on Python coding standards.

Index: gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.326
diff -u -p -r1.326 gdbint.texinfo
--- gdbint.texinfo	22 Jun 2010 05:03:19 -0000	1.326
+++ gdbint.texinfo	20 Oct 2010 22:49:46 -0000
@@ -83,7 +83,8 @@ as the mechanisms that adapt @value{GDBN
 * Target Vector Definition::
 * Native Debugging::
 * Support Libraries::
-* Coding::
+* Coding Standards::
+* Misc Guidelines::
 * Porting GDB::
 * Versions and Branches::
 * Start of New Year Procedure::
@@ -1322,9 +1323,9 @@ be signaled.
 
 @deftypefun {struct cleanup *} make_cleanup_ui_out_tuple_begin_end (struct ui_out *@var{uiout}, const char *@var{id})
 This function first opens the tuple and then establishes a cleanup
-(@pxref{Coding, Cleanups}) to close the tuple.  It provides a convenient
-and correct implementation of the non-portable@footnote{The function
-cast is not portable ISO C.} code sequence:
+(@pxref{Misc Guidelines, Cleanups}) to close the tuple.
+It provides a convenient and correct implementation of the
+non-portable@footnote{The function cast is not portable ISO C.} code sequence:
 @smallexample
 struct cleanup *old_cleanup;
 ui_out_tuple_begin (uiout, "...");
@@ -1349,7 +1350,8 @@ be signaled.
 
 @deftypefun {struct cleanup *} make_cleanup_ui_out_list_begin_end (struct ui_out *@var{uiout}, const char *@var{id})
 Similar to @code{make_cleanup_ui_out_tuple_begin_end}, this function
-opens a list and then establishes cleanup (@pxref{Coding, Cleanups})
+opens a list and then establishes cleanup
+(@pxref{Misc Guidelines, Cleanups})
 that will close the list.
 @end deftypefun
 
@@ -5756,9 +5758,235 @@ Binary search the array.
 
 @section include
 
-@node Coding
+@node Coding Standards
 
-@chapter Coding
+@chapter Coding Standards
+@cindex coding standards
+
+@section @value{GDBN} C Coding Standards
+
+@value{GDBN} follows the GNU coding standards, as described in
+@file{etc/standards.texi}.  This file is also available for anonymous
+FTP from GNU archive sites.  @value{GDBN} takes a strict interpretation
+of the standard; in general, when the GNU standard recommends a practice
+but does not require it, @value{GDBN} requires it.
+
+@value{GDBN} follows an additional set of coding standards specific to
+@value{GDBN}, as described in the following sections.
+
+@subsection ISO C
+
+@value{GDBN} assumes an ISO/IEC 9899:1990 (a.k.a.@: ISO C90) compliant
+compiler.
+
+@value{GDBN} does not assume an ISO C or POSIX compliant C library.
+
+@subsection Formatting
+
+@cindex source code formatting
+The standard GNU recommendations for formatting must be followed
+strictly.
+
+A function declaration should not have its name in column zero.  A
+function definition should have its name in column zero.
+
+@smallexample
+/* Declaration */
+static void foo (void);
+/* Definition */
+void
+foo (void)
+@{
+@}
+@end smallexample
+
+@emph{Pragmatics: This simplifies scripting.  Function definitions can
+be found using @samp{^function-name}.}
+
+There must be a space between a function or macro name and the opening
+parenthesis of its argument list (except for macro definitions, as
+required by C).  There must not be a space after an open paren/bracket
+or before a close paren/bracket.
+
+While additional whitespace is generally helpful for reading, do not use
+more than one blank line to separate blocks, and avoid adding whitespace
+after the end of a program line (as of 1/99, some 600 lines had
+whitespace after the semicolon).  Excess whitespace causes difficulties
+for @code{diff} and @code{patch} utilities.
+
+Pointers are declared using the traditional K&R C style:
+
+@smallexample
+void *foo;
+@end smallexample
+
+@noindent
+and not:
+
+@smallexample
+void * foo;
+void* foo;
+@end smallexample
+
+@subsection Comments
+
+@cindex comment formatting
+The standard GNU requirements on comments must be followed strictly.
+
+Block comments must appear in the following form, with no @code{/*}- or
+@code{*/}-only lines, and no leading @code{*}:
+
+@smallexample
+/* Wait for control to return from inferior to debugger.  If inferior
+   gets a signal, we may decide to start it up again instead of
+   returning.  That is why there is a loop in this function.  When
+   this function actually returns it means the inferior should be left
+   stopped and @value{GDBN} should read more commands.  */
+@end smallexample
+
+(Note that this format is encouraged by Emacs; tabbing for a multi-line
+comment works correctly, and @kbd{M-q} fills the block consistently.)
+
+Put a blank line between the block comments preceding function or
+variable definitions, and the definition itself.
+
+In general, put function-body comments on lines by themselves, rather
+than trying to fit them into the 20 characters left at the end of a
+line, since either the comment or the code will inevitably get longer
+than will fit, and then somebody will have to move it anyhow.
+
+@subsection C Usage
+
+@cindex C data types
+Code must not depend on the sizes of C data types, the format of the
+host's floating point numbers, the alignment of anything, or the order
+of evaluation of expressions.
+
+@cindex function usage
+Use functions freely.  There are only a handful of compute-bound areas
+in @value{GDBN} that might be affected by the overhead of a function
+call, mainly in symbol reading.  Most of @value{GDBN}'s performance is
+limited by the target interface (whether serial line or system call).
+
+However, use functions with moderation.  A thousand one-line functions
+are just as hard to understand as a single thousand-line function.
+
+@emph{Macros are bad, M'kay.}
+(But if you have to use a macro, make sure that the macro arguments are
+protected with parentheses.)
+
+@cindex types
+
+Declarations like @samp{struct foo *} should be used in preference to
+declarations like @samp{typedef struct foo @{ @dots{} @} *foo_ptr}.
+
+@subsection Function Prototypes
+@cindex function prototypes
+
+Prototypes must be used when both @emph{declaring} and @emph{defining}
+a function.  Prototypes for @value{GDBN} functions must include both the
+argument type and name, with the name matching that used in the actual
+function definition.
+
+All external functions should have a declaration in a header file that
+callers include, except for @code{_initialize_*} functions, which must
+be external so that @file{init.c} construction works, but shouldn't be
+visible to random source files.
+
+Where a source file needs a forward declaration of a static function,
+that declaration must appear in a block near the top of the source file.
+
+@subsection File Names
+
+Any file used when building the core of @value{GDBN} must be in lower
+case.  Any file used when building the core of @value{GDBN} must be 8.3
+unique.  These requirements apply to both source and generated files.
+
+@emph{Pragmatics: The core of @value{GDBN} must be buildable on many
+platforms including DJGPP and MacOS/HFS.  Every time an unfriendly file
+is introduced to the build process both @file{Makefile.in} and
+@file{configure.in} need to be modified accordingly.  Compare the
+convoluted conversion process needed to transform @file{COPYING} into
+@file{copying.c} with the conversion needed to transform
+@file{version.in} into @file{version.c}.}
+
+Any file non 8.3 compliant file (that is not used when building the core
+of @value{GDBN}) must be added to @file{gdb/config/djgpp/fnchange.lst}.
+
+@emph{Pragmatics: This is clearly a compromise.}
+
+When @value{GDBN} has a local version of a system header file (ex
+@file{string.h}) the file name based on the POSIX header prefixed with
+@file{gdb_} (@file{gdb_string.h}).  These headers should be relatively
+independent: they should use only macros defined by @file{configure},
+the compiler, or the host; they should include only system headers; they
+should refer only to system types.  They may be shared between multiple
+programs, e.g.@: @value{GDBN} and @sc{gdbserver}.
+
+For other files @samp{-} is used as the separator.
+
+@subsection Include Files
+
+A @file{.c} file should include @file{defs.h} first.
+
+A @file{.c} file should directly include the @code{.h} file of every
+declaration and/or definition it directly refers to.  It cannot rely on
+indirect inclusion.
+
+A @file{.h} file should directly include the @code{.h} file of every
+declaration and/or definition it directly refers to.  It cannot rely on
+indirect inclusion.  Exception: The file @file{defs.h} does not need to
+be directly included.
+
+An external declaration should only appear in one include file.
+
+An external declaration should never appear in a @code{.c} file.
+Exception: a declaration for the @code{_initialize} function that
+pacifies @option{-Wmissing-declaration}.
+
+A @code{typedef} definition should only appear in one include file.
+
+An opaque @code{struct} declaration can appear in multiple @file{.h}
+files.  Where possible, a @file{.h} file should use an opaque
+@code{struct} declaration instead of an include.
+
+All @file{.h} files should be wrapped in:
+
+@smallexample
+#ifndef INCLUDE_FILE_NAME_H
+#define INCLUDE_FILE_NAME_H
+header body
+#endif
+@end smallexample
+
+@section @value{GDBN} Python Coding Standards
+
+@value{GDBN} follows the published @code{Python} coding standards in
+@uref{http://www.python.org/dev/peps/pep-0008/, @code{PEP008}}.
+
+In addition, the guidelines in the
+@uref{http://google-styleguide.googlecode.com/svn/trunk/pyguide.html,
+Google Python Style Guide} are also followed where they do not
+conflict with @code{PEP008}.
+
+@subsection @value{GDBN}-specific exceptions
+
+There are a few exceptions to the published standards.
+They exist mainly for consistency with the @code{C} standards.
+
+@c It is expected that there are a few more exceptions,
+@c so we use itemize here.
+
+@itemize @bullet
+
+@item
+Use @code{FIXME} instead of @code{TODO}.
+
+@end itemize
+
+@node Misc Guidelines
+
+@chapter Misc Guidelines
 
 This chapter covers topics that are lower-level than the major
 algorithms of @value{GDBN}.
@@ -5995,28 +6223,7 @@ finish by printing a newline, to flush t
 to unfiltered (@code{printf}) output.  Symbol reading routines that
 print warnings are a good example.
 
-@section @value{GDBN} Coding Standards
-@cindex coding standards
-
-@value{GDBN} follows the GNU coding standards, as described in
-@file{etc/standards.texi}.  This file is also available for anonymous
-FTP from GNU archive sites.  @value{GDBN} takes a strict interpretation
-of the standard; in general, when the GNU standard recommends a practice
-but does not require it, @value{GDBN} requires it.
-
-@value{GDBN} follows an additional set of coding standards specific to
-@value{GDBN}, as described in the following sections.
-
-
-@subsection ISO C
-
-@value{GDBN} assumes an ISO/IEC 9899:1990 (a.k.a.@: ISO C90) compliant
-compiler.
-
-@value{GDBN} does not assume an ISO C or POSIX compliant C library.
-
-
-@subsection Memory Management
+@section Memory Management
 
 @value{GDBN} does not use the functions @code{malloc}, @code{realloc},
 @code{calloc}, @code{free} and @code{asprintf}.
@@ -6054,7 +6261,7 @@ functions such as @code{sprintf} are ver
 errors.}
 
 
-@subsection Compiler Warnings
+@section Compiler Warnings
 @cindex compiler warnings
 
 With few exceptions, developers should avoid the configuration option
@@ -6109,124 +6316,7 @@ currently too noisy to enable with @samp
 
 @end table
 
-@subsection Formatting
-
-@cindex source code formatting
-The standard GNU recommendations for formatting must be followed
-strictly.
-
-A function declaration should not have its name in column zero.  A
-function definition should have its name in column zero.
-
-@smallexample
-/* Declaration */
-static void foo (void);
-/* Definition */
-void
-foo (void)
-@{
-@}
-@end smallexample
-
-@emph{Pragmatics: This simplifies scripting.  Function definitions can
-be found using @samp{^function-name}.}
-
-There must be a space between a function or macro name and the opening
-parenthesis of its argument list (except for macro definitions, as
-required by C).  There must not be a space after an open paren/bracket
-or before a close paren/bracket.
-
-While additional whitespace is generally helpful for reading, do not use
-more than one blank line to separate blocks, and avoid adding whitespace
-after the end of a program line (as of 1/99, some 600 lines had
-whitespace after the semicolon).  Excess whitespace causes difficulties
-for @code{diff} and @code{patch} utilities.
-
-Pointers are declared using the traditional K&R C style:
-
-@smallexample
-void *foo;
-@end smallexample
-
-@noindent
-and not:
-
-@smallexample
-void * foo;
-void* foo;
-@end smallexample
-
-@subsection Comments
-
-@cindex comment formatting
-The standard GNU requirements on comments must be followed strictly.
-
-Block comments must appear in the following form, with no @code{/*}- or
-@code{*/}-only lines, and no leading @code{*}:
-
-@smallexample
-/* Wait for control to return from inferior to debugger.  If inferior
-   gets a signal, we may decide to start it up again instead of
-   returning.  That is why there is a loop in this function.  When
-   this function actually returns it means the inferior should be left
-   stopped and @value{GDBN} should read more commands.  */
-@end smallexample
-
-(Note that this format is encouraged by Emacs; tabbing for a multi-line
-comment works correctly, and @kbd{M-q} fills the block consistently.)
-
-Put a blank line between the block comments preceding function or
-variable definitions, and the definition itself.
-
-In general, put function-body comments on lines by themselves, rather
-than trying to fit them into the 20 characters left at the end of a
-line, since either the comment or the code will inevitably get longer
-than will fit, and then somebody will have to move it anyhow.
-
-@subsection C Usage
-
-@cindex C data types
-Code must not depend on the sizes of C data types, the format of the
-host's floating point numbers, the alignment of anything, or the order
-of evaluation of expressions.
-
-@cindex function usage
-Use functions freely.  There are only a handful of compute-bound areas
-in @value{GDBN} that might be affected by the overhead of a function
-call, mainly in symbol reading.  Most of @value{GDBN}'s performance is
-limited by the target interface (whether serial line or system call).
-
-However, use functions with moderation.  A thousand one-line functions
-are just as hard to understand as a single thousand-line function.
-
-@emph{Macros are bad, M'kay.}
-(But if you have to use a macro, make sure that the macro arguments are
-protected with parentheses.)
-
-@cindex types
-
-Declarations like @samp{struct foo *} should be used in preference to
-declarations like @samp{typedef struct foo @{ @dots{} @} *foo_ptr}.
-
-
-@subsection Function Prototypes
-@cindex function prototypes
-
-Prototypes must be used when both @emph{declaring} and @emph{defining}
-a function.  Prototypes for @value{GDBN} functions must include both the
-argument type and name, with the name matching that used in the actual
-function definition.
-
-All external functions should have a declaration in a header file that
-callers include, except for @code{_initialize_*} functions, which must
-be external so that @file{init.c} construction works, but shouldn't be
-visible to random source files.
-
-Where a source file needs a forward declaration of a static function,
-that declaration must appear in a block near the top of the source file.
-
-
-@subsection Internal Error Recovery
+@section Internal Error Recovery
 
 During its execution, @value{GDBN} can encounter two types of errors.
 User errors and internal errors.  User errors include not only a user
@@ -6245,76 +6335,11 @@ the code detected a user error, recovere
 @code{warning} or the code failed to correctly recover from the user
 error and issued an @code{internal_error}.}
 
-@subsection Command Names
+@section Command Names
 
 GDB U/I commands are written @samp{foo-bar}, not @samp{foo_bar}.
 
-@subsection File Names
-
-Any file used when building the core of @value{GDBN} must be in lower
-case.  Any file used when building the core of @value{GDBN} must be 8.3
-unique.  These requirements apply to both source and generated files.
-
-@emph{Pragmatics: The core of @value{GDBN} must be buildable on many
-platforms including DJGPP and MacOS/HFS.  Every time an unfriendly file
-is introduced to the build process both @file{Makefile.in} and
-@file{configure.in} need to be modified accordingly.  Compare the
-convoluted conversion process needed to transform @file{COPYING} into
-@file{copying.c} with the conversion needed to transform
-@file{version.in} into @file{version.c}.}
-
-Any file non 8.3 compliant file (that is not used when building the core
-of @value{GDBN}) must be added to @file{gdb/config/djgpp/fnchange.lst}.
-
-@emph{Pragmatics: This is clearly a compromise.}
-
-When @value{GDBN} has a local version of a system header file (ex
-@file{string.h}) the file name based on the POSIX header prefixed with
-@file{gdb_} (@file{gdb_string.h}).  These headers should be relatively
-independent: they should use only macros defined by @file{configure},
-the compiler, or the host; they should include only system headers; they
-should refer only to system types.  They may be shared between multiple
-programs, e.g.@: @value{GDBN} and @sc{gdbserver}.
-
-For other files @samp{-} is used as the separator.
-
-
-@subsection Include Files
-
-A @file{.c} file should include @file{defs.h} first.
-
-A @file{.c} file should directly include the @code{.h} file of every
-declaration and/or definition it directly refers to.  It cannot rely on
-indirect inclusion.
-
-A @file{.h} file should directly include the @code{.h} file of every
-declaration and/or definition it directly refers to.  It cannot rely on
-indirect inclusion.  Exception: The file @file{defs.h} does not need to
-be directly included.
-
-An external declaration should only appear in one include file.
-
-An external declaration should never appear in a @code{.c} file.
-Exception: a declaration for the @code{_initialize} function that
-pacifies @option{-Wmissing-declaration}.
-
-A @code{typedef} definition should only appear in one include file.
-
-An opaque @code{struct} declaration can appear in multiple @file{.h}
-files.  Where possible, a @file{.h} file should use an opaque
-@code{struct} declaration instead of an include.
-
-All @file{.h} files should be wrapped in:
-
-@smallexample
-#ifndef INCLUDE_FILE_NAME_H
-#define INCLUDE_FILE_NAME_H
-header body
-#endif
-@end smallexample
-
-
-@subsection Clean Design and Portable Implementation
+@section Clean Design and Portable Implementation
 
 @cindex design
 In addition to getting the syntax right, there's the little question of
@@ -6448,7 +6473,6 @@ All debugging code must be controllable 
 messages.  Use @code{fprintf_unfiltered(gdb_stdlog, ...}.  Do not use
 @code{#ifdef DEBUG}.
 
-
 @node Porting GDB
 
 @chapter Porting @value{GDBN}

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-20 22:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-19 15:53 [doc RFA] Clean up coding standards, add python coding standards Doug Evans
2010-10-19 18:45 ` Eli Zaretskii
2010-10-20 22:56   ` Doug Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).