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.
char ∗s
, not char∗ s
).else if
on a single linesizeof(variable)
instead of sizeof(typeofvariable)
so that a change in the type of variable
doesn't require a change in the use of sizeof
sizeof(*pointer_to_object)
rather than sizeof(object_type)
to allow type change flexibilityenum
rather than #define
for that limited setstrtok_r()
rather than strtok()
(even on Windows -- PBS's win.h
header file redefines strtok_r
to use strtok_s
on Windows platforms)const
whenever appropriate#define
and macros constructively#define
constructs.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 ); |
void
and void *
where appropriate (e.g. when a function returns no value or with pointers suitable for coercion to any type of object)static
for scoping as appropriateRight:
void example( void ) { } |
Wrong:
void example( void ) { } |
Right:
void example( void ) { } |
Wrong:
void example( void ) { } |
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 ; } |
void
*Right:
void * lim_dup_liminfo( void *p) |
You don't need to cast return values of functions that return void *
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 */ { |
(void)
rather than () in the signatureRight:
func (void ) |
Wrong:
func() |
extern
when declaring a function or variable in a header file#ifndef _FOO_H #define _FOO_H |
and ending with:
#endif /* _FOO_H */ |
#ifdef __cplusplus extern "C" { #endif |
and the include file guard epilogue must be immediately preceded by
#ifdef __cplusplus } #endif |
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. */ |
/
of a comment to the same level as the code to which it applies_
in symbol names rather than "camelCase"enum
and #define
enum"
and "#define"
in uppercaseenum
and #define
pbs_
prefix for public interfaces_t
suffix when defining a typedef _g
suffix when defining a globalp
tmp
suffix or prefixRight:
struct foo { ... }; typedef struct foo foo_t; |
Right:
struct a { unsigned int a_flags; unsigned int a_type; }; |
Wrong:
struct a { unsigned int flags; unsigned int type; }; |
for
loops as while
loops; write infinite loops using for (;;)
rather than while (1)
. Right:
for ( ptask = (struct work_task *)GET_NEXT(ptask->wt_linkall)) |
Wrong:
ptask = (struct work_task *)GET_NEXT(task_list_event); |
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 ; } |
\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.do
, for
, while
, return
, switch
) but not after function names, sizeof
, casts, or parentheses. &
, .
, *
, ->
) or parenthesesdo { ... } while (<conditional>); |
for (;;) { ... } |
switch (<expression>) { case 1 : ... break ; case 2: ... } |
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 */ ... } } |
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(ptask->wt_linkall)) { if ((ptask->wt_type == WORK_Deferred_Child) && #ifdef WIN32 ((HANDLE)ptask->wt_event == pid)) # else (ptask->wt_event == pid)) #endif { ... } } } } |
Follow the C Coding Standards, then look here for C++ specific guidelines.
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)
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);
}
pbs_config.h if needed
System and STL headers
pbs headers
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.
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;
}
NULL is often just defined 0, nullptr is a pointer type pointing to the address 0x0.
// Bad
int *p = NULL;
// Good
int *p = nullptr;
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>();
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(" ");
You can find an example of unique pointers here: https://openpbs.atlassian.net/wiki/spaces/PD/pages/2172551169/Unique+Pointer+Example
You can find an example of these OOP features here: https://openpbs.atlassian.net/wiki/spaces/PD/pages/2191458305
#!
) construct, and that #!/bin/sh
suffices.
|
Instead, break the function manually as
|
astyle
)astyle
is a formatter for C, C++, C#, and Java
Tool to automatically format C/C++/Objective-C code, so that developers don't need to worry about style issues during code reviews.
Tool for indenting and formatting C
Secure Programming Lint is an enhanced version of lint
that provides lightweight static analysis of C programs.