/
OpenPBS Coding Standards

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.

Related content

Analysis: Build PBS using CMake
Analysis: Build PBS using CMake
More like this
Project Documentation Home 2
Project Documentation Home 2
More like this
OpenPBS Design Document Guidelines
OpenPBS Design Document Guidelines
More like this
Using the PBS IFL API via Python
Using the PBS IFL API via Python
More like this
Pre-built Packages and Tested Platforms
Pre-built Packages and Tested Platforms
More like this
Contributing to OpenPBS
Contributing to OpenPBS
More like this