OpenPBS Coding Standards

Why Coding Standards?

The code you contribute to PBS will be read by many different people, and will stick around for a long time.  Make it easy to read, learn, and review your code by following the coding standards, and by using standardized interfaces instead of nonstandard.


Contents


C Coding Style

  • Use ANSI C style; not K&R
  • Do not decorate types (i.e. use char ∗s, not char∗ s).
  • put else if on a single line
  • use sizeof(variable) instead of sizeof(typeofvariable) so that a change in the type of variable doesn't require a change in the use of sizeof
  • when allocating space for an object, use sizeof(*pointer_to_object) rather than sizeof(object_type) to allow type change flexibility
  • match a program's usage statement to its man page's SYNOPSIS
  • if a variable can take on a limited number of values, or a function can return a limited number of values, choose enum rather than #define for that limited set
  • use strtok_r() rather than strtok() (even on Windows -- PBS's win.h header file redefines strtok_r to use strtok_s on Windows platforms)
  • when you discover non-ANSI-compliant code, fix it
  • use const whenever appropriate
  • You don't need braces for single statements

Macros

  • Make macro names all upper-case
  • Always parenthesize macro arguments in definitions
  • If a macro's definition could affect code flow (for example, it expands to either multiple statements or none), surround its use with braces
  • Use #define and macros constructively
  • Some naming guidelines may lead to long names, and crazily long lines of code. You can shorten unions within structures via #define constructs.
  • You can improve code readability with macros.  Compare these
eligibletime += subj->ji_wattr[(int)JOB_ATR_sample_starttime].at_val.at_long -
parent->ji_wattr[(int)JOB_ATR_sample_starttime].at_val.at_long;

and

eligibletime += JATTR(subj, sample_starttime, long) - JATTR(parent, sample_starttime, long);

Functions

  • Put private function declarations at the top of a source file
  • use void and void * where appropriate (e.g. when a function returns no value or with pointers suitable for coercion to any type of object)
  • Indent local variable definitions one level. You can put multiple definitions in a single statement
  • You don't need to align variable names and trailing same-line comments
  • Indent continuation lines of a function's signature in the function's definition or declaration consistently, so that they are easy to read
  • use static for scoping as appropriate

Function Opening Braces

  • Put a function's opening brace in column one
  • Right:

    void
    example(void)
    {
    }

    Wrong:

    void
    example(void) {
    }

Function Type and Name Positioning

  • Separate a function's type and name by a newline (that is, both the type and name must start in column one). 
  • Right:

    void
    example(void)
    {
    }

    Wrong:

    void example(void)
    {
    }

Returning a void

  • Right:

    extern void pbs_statfree(struct batch_status *bsp);

    If a function does not return a value, don't add a return line at the end

    Right:

    static void
    del_event(event_info *ep)
    {
            ...
    }

    Wrong:

    static void
    del_event(event_info *ep)
    {
            ...
            return;
    }

Returning a void *

  • Right:

    void *
    lim_dup_liminfo(void *p)

    You don't need to cast return values of functions that return void *

Place function arguments inside function signature

  • Right:

    static char *
    resc_to_string(attribute *pattr, char *buf, int buflen)
    {

    Wrong:

    static char *resc_to_string(pattr, buf, buflen)
    attribute *pattr;       /* the attribute to convert */
    char      *buf;         /* the buffer into which to convert */
    int        buflen;      /* the length of the above buffer */
    {
  • If there are no arguments, use (void) rather than () in the signature
  • Right:

    func(void)

    Wrong:

    func()

Header Files

  • Begin with a licensing notice
  • use extern when declaring a function or variable in a header file

Use File Guards

  • Include header file guards, starting with:


    #ifndef _FOO_H
    #define _FOO_H

    and ending with:

    #endif  /* _FOO_H */

Use C++ Naming Guards

  • Include a C++ namespace guard
    The include file guard prologue must be immediately followed by


    #ifdef  __cplusplus
    extern "C" {
    #endif

    and the include file guard epilogue must be immediately preceded by


    #ifdef  __cplusplus
    }
    #endif

Include Description of Purpose

  • Include a description of the file's purpose. For example (from pbs/src/include/pbs_db.h),


    /**
    * @file    pbs_db.h
    *
    * @brief
    * PBS database interface. (Function declarations and structures)
    *
    * This header file contains functions to access the PBS data store
    * Only these functions should be used by PBS code. Actual implementation
    * of these functions are database-specific and are implemented in Libdb.
    */


Comments

  • Use correct grammar, punctuation, and spelling
  • Don't use C++ style comments (i.e. / /)
  • indent the opening / of a comment to the same level as the code to which it applies

Multiline Comments

  • start a multiline comment with /*, indented to the same level as the code to which it applies
  • continue each line with a * aligned with the initial *
  • end with a */ on a line by itself


Doxygen Comments

  • Write a doxygen block comment just before the definition of a function describing:
    • what the function does
    • function parameters
    • function return values


Naming

  • use helpful function and variable naming, e.g. "can_run_job(pjob)" instead of "check_job(pjob)"
  • use lower case and _ in symbol names rather than "camelCase"
  • if it makes sense, use a similar naming convention for related enum and #define
  • Put items after "enum" and "#define" in uppercase
  • Use a distinctive prefix for enum and #define
  • avoid the Windows (i.e. Hungarian) style of naming variables; do not encode length or type information in the name

Prefixes and Suffixes in Names

  • use the pbs_ prefix for public interfaces
  • use an _t suffix when defining a typedef
  • use an _g suffix when defining a global
  • when sensible, start names of pointers with p
  • denote temporary variables with a tmp suffix or prefix


Naming Typedefs

  • Right:

    struct foo {
            ...
    };
    typedef struct foo foo_t;

Naming Structure and Union Members 

  • Right:

    struct a {
            unsigned int    a_flags;
            unsigned int    a_type;
    };

    Wrong:

    struct a {
            unsigned int    flags;
            unsigned int    type;
    };


Looping

  • Do not disguise for loops as while loops; write infinite loops using for (;;) rather than while (1).

    Right:

    for (ptask = (struct work_task *)GET_NEXT(task_list_event);ptask;
         ptask = (struct work_task *)GET_NEXT(ptask->wt_linkall))

    Wrong:

    ptask = (struct work_task *)GET_NEXT(task_list_event);
    while (ptask) {
            ...
            ptask = (struct work_task *)GET_NEXT(ptask->wt_linkall);
    }
  • In the following, the while statement is a red herring.  This is really a for loop and should be written that way


    i = 1;
    while (i < 10) {
            printf("hello, world\n");
            i += 1;
    }

Formatting

Indentation

  • Use leading tabs, not spaces, for indentation (it's easier to customize tabs to your personal style)

Braces and Parentheses

  • make sure braces, parentheses, etc. match, even in the presence of preprocessor directives
  • Use parentheses anywhere it makes it easier for the reader

Line Termination

  • end text files with a newline
  • use UNIX-style (\n) line termination rather than DOS-style (\r\n). Check this yourself; do not rely on tools of a source code management system to do this for you.

Continuing Lines

  • when you continue a line, break it after, not before, an operator
  • indent continuation lines sensibly and consistently; make it easy to read
  • Do not use white space around parentheses.


White Space

  • put spaces after keywords (e.g. do, for, while, return, switch) but not after function names, sizeof, casts, or parentheses. 
  • place one space on either side of the ‘=’ when assigning variable values, and for operations including '&&', ‘+’, ‘*’, ‘<’, ‘>’, etc.
  • do not end lines with white space
  • do not use white space between function names and arguments or around references and dereferences (&, ., *, ->) or parentheses
  • do not try to align variable names or types with whitespace when declaring multiple variables. Use a single space between variable names and types.
  • do not use spaces between increment/decrement operators or pointer operations such as ‘*’, ‘&’ and ‘->’.

Examples of Braces and White Spaces

  • Right:
    do {
            ...
    } while (<conditional>);


    for (;;) {
            ...
    }


    switch (<expression>) {
    case 1:
            ...
            break;
    case 2:
            ...
            break;
    }


Balance Braces and Parentheses

  • Use parentheses anywhere it makes it easier for the reader
  • Make sure braces, parentheses, etc. match, even in the presence of preprocessor directives: put opening curly brace after #endif so that your editor won't see more opening braces than closing.
  • Right (braces are balanced):

    #ifdef WIN32
            for (i = 0; i <= max_connection && n; i++)/*  for select() in WIN32 */
    #else
            for (i = 0; i <= maxfdx && n; i++)/* for poll() in Unix */
    #endif
                     {
                    if (selpoll_fd_isset(i)) { /* this socket has data */
                                               n--; /* decrement the no. of events */
                                               ...
                    }
            }

    Wrong (braces are unbalanced -- note one in each for loop):

    #ifdef WIN32
            for (i = 0; i <= max_connection && n; i++) { /*  for select() in WIN32 */
    #else
            for (i = 0; i <= maxfdx && n; i++) { /* for poll() in Unix */
    #endif
                    if (selpoll_fd_isset(i)) { /* this socket has data */
                                               n--; /* decrement the no. of events */
                                               ...
                    }
            }
  • Balance your parentheses inside your #ifdefs.  Don't close parentheses inside an #ifdef if it was opened outside of the #ifdef.
  • Right:
    static void reap_child(void)
    {
    struct work_task *ptask;
            for (ptask = (struct work_task *)GET_NEXT(task_list_event);ptask;
                 ptask = (struct work_task *)GET_NEXT(ptask->wt_linkall)) {
                    if ((ptask->wt_type == WORK_Deferred_Child) &&
    #ifdef WIN32
                       ((HANDLE)ptask->wt_event == pid)
    #else
                       (ptask->wt_event == pid)
    #endif
                                            )
                            {
                            ...
                            }
                    }
            }
    }

    Wrong:

    static void reap_child(void)
    {
    struct work_task *ptask;
            for (ptask = (struct work_task *)GET_NEXT(task_list_event);ptask;
                 ptask = (struct work_task *)GET_NEXT(ptask->wt_linkall)) {
                    if ((ptask->wt_type == WORK_Deferred_Child) &&
    #ifdef WIN32
                        ((HANDLE)ptask->wt_event == pid))
    #else
                        (ptask->wt_event == pid))
    #endif
                            {
                            ...
                            }
                    }
            }
    }

C++ Standards

Follow the C Coding Standards, then look here for C++ specific guidelines.

File Extensions

Source files should use .cpp
Header files should use .hpp

Because we a mixed C/C++ project, it is helpful to know which header files we can use in a C file. Therefore, a C header should have a different extension than a C++ header.

.hpp is chosen because it easily attributed to .cpp files.

From the Boost FAQ:

File extensions communicate the "type" of the file, both to humans and to computer programs. The '.h' extension is used for C header files, and therefore communicates the wrong thing about C++ header files... Using '.hpp' unambiguously identifies it as C++ header file, and works well in actual practice. (Rainer Deyke)

Whitespace

We use tabs to be consistent with our C code.

Because we use tabs, we can’t use partial indents.
We don’t want to have a ton of horizontal white space making our lines longer than it has to be.

Classes will not have an indent before public, protected and private

// Bad
class MyClass {
  public:
    MyClass();
    ~MyClass();
  private:
    int count;
};

// Good
class MyClass {
public:
  MyClass();
  ~MyClass();
private:
  int count;
};

Namespaces will not cause a level of indentation

namespace pbs {
// Bad
  int add(int x, int y);
// Good
int add(int x, int y);
}

Names and Orders of Includes

  1. pbs_config.h if needed

  2. System and STL headers

  3. pbs headers

Some good things to keep in mind

Casting

Do not use C-style casts; instead use C++ style casts.

// Bad
int *p = (int *)malloc(sizeof(int));
// Good
int *p = static_cast<int *>(malloc(sizeof(int)));

Note: Void pointers can't be assigned to non-void pointers without a cast.

Use const char * whenever possible

If you're not changing a char * argument, use const in the function definition

// Bad
void
logmsg(int sev, char *msg)
{
  printf("%d\t%s\n", sev, msg);
  return;
}
// Good
void
logmsg(int sev, const char *msg)
{
  printf("%d\t%s\n", sev, msg);
  return;
}

Prefer nullptr to NULL

NULL is often just defined 0, nullptr is a pointer type pointing to the address 0x0.

// Bad
int *p = NULL;
// Good
int *p = nullptr;

auto is awesome

A lot of types can be very long strings, such as classes with templates. auto lets you save some typing by deducing the type from the right-hand-side.

// bad
std::vector<MyClass> list = std::vector<MyClass>();
// good
auto list2 = std::vector<MyClass>();

Namespaces

Using using namespace <name> is not a good idea, as it can pollute the global namespace. Instead, use the namespace prefix.

// Bad
using namespace std;
auto vec = vector<int>();
// Good
auto vec = std::vector<int>();

If you’re in the pbs namespace, don’t create another namespace prefixed with pbs_

// Bad
namespace pbs {
namespace pbs_util {
int count_spaces(const char *);
}
}
int spaces = pbs::pbs_util::count_spaces("    ");

// Good
namespace pbs{
namespace util {
int count_spaces(const char *);
}
}
int spaces = pbs::util::count_spaces("    ");

Unique Pointers

You can find an example of unique pointers here: https://openpbs.atlassian.net/wiki/spaces/PD/pages/2172551169/Unique+Pointer+Example

Inheritance/Polymorphism

You can find an example of these OOP features here: https://openpbs.atlassian.net/wiki/spaces/PD/pages/2191458305

Shell Scripts

  • PBS assumes that its shell scripts can use the executable script (#!) construct, and that #!/bin/sh suffices.


Python Coding Style

  • We follow the PEP8 standard. You can check whether your python code adheres to PEP8 by running the pycodestyle program on your .py file.
  • In your function docstring, say what the function does.  For example, "Reconfirm degraded reservation if there are sufficient nodes".  Be succinct but describe the function's purpose completely.  A huge docstring means your function needs to be split up.
  • Use descriptive variable and function names.
  • Explain why you are doing it.  For example, you created 3 queues but are submitting jobs to just one of them. While it's obvious to you that you are submitting jobs to a routing queue that routes jobs to the other two queues, it may not be obvious to others.  Explain why (not what) instead of making your reader figure it out. Example:
    • Right: "Submit J2 to preempt J1 since J2 is in an express queue."
    • Wrong: "Submit second job."
  • Explain each step of a multi-step test, if necessary
  • Use meaningful debugging log messages that explain in enough detail for someone else to figure out what's happening
  • Use APIs.  For example, use os.path.join() instead of “+” to join paths, and use self.du.get_tempdir() to get path to temp directory instead of directly using '/tmp/'.
  • Be kind to your test runner. If you are going to have a long sleep or offset, put a log message explaining why. This explains why to the test runner as well as the test reader.
  • Import only the modules that are necessary
  • Break long lines manually instead of using autopep8: autopep8 might break a long function prototype as: 
    self.du.chown(
                     svrname, path=userhome, uid=username, gid=grpname, sudo=True)


    Instead, break the function manually as

    self.du.chown(svrname, path=userhome, uid=username,
                  gid=grpname, sudo=True)

Tools


Artistic Style (astyle)

astyle is a formatter for C, C++, C#, and Java

clang-format

Tool to automatically format C/C++/Objective-C code, so that developers don't need to worry about style issues during code reviews.

GNU Indent

Tool for indenting and formatting C

Splint

Secure Programming Lint is an enhanced version of lint that provides lightweight static analysis of C programs.