Skip to content

Latest commit

 

History

History
1398 lines (988 loc) · 22.5 KB

CONTRIBUTING.md

File metadata and controls

1398 lines (988 loc) · 22.5 KB

Intro

Frida is composed of many sub-projects and its code spans across many different languages, such as C, C++, Vala, JavaScript, TypeScript, Python, assembly, etc., and the coding conventions may vary across any combination of project/language.

The rules presented here are not there because of @oleavr's OCD: they're about keeping the codebase readable and maintainable, and give the code a consistent structure as it grows. That helps every contributor to easily orientate in it.

Moreover, reading this doc helps save everyone's time when it comes to get a PR reviewed.

General rules

Rules in this section apply in all cases, regardless of the programming language or the project owning the code. Examples are in pseudo-javascript for brevity (with some exceptions).

Comments vs. naming

Rule

Only use comments for communicating what cannot be conveyed by the code itself through meaningful variable and function names, and splitting out logic into separate functions.

NOTE: names should be long enough to be meaningful, but not ridiculously long.

Wrong

// Init the dyld start address with current program counter
a = (instance->cpu_type == GUM_CPU_ARM64) ? __darwin_arm_thread_state64_get_pc (state.ts_64) : state.ts_32.__pc;
// dyld header is initially zero
b = 0;
// set the search granularity to 4k
c = 4096;

for (dyld_chunk = (a & (c - 1)) == 0 ? (a - c) : (a & ~(c - 1));
    b == 0;
    dyld_chunk -= c)
{
	...
}

Correct

dyld_start = (instance->cpu_type == GUM_CPU_ARM64) ? __darwin_arm_thread_state64_get_pc (state.ts_64) : state.ts_32.__pc;
dyld_header = 0;
dyld_granularity = 4096;

for (dyld_chunk = (dyld_start & (dyld_granularity - 1)) == 0 ? (dyld_start - dyld_granularity) : (dyld_start & ~(dyld_granularity - 1));
    dyld_header == 0;
    dyld_chunk -= dyld_granularity)
{
	...
}

Example

Here's a really useful comment.

/* Compiled from helpers/upload-listener.c */
private const uint8[] UPLOAD_LISTENER_CODE = {
	0xff, 0x43, 0x01, 0xd1, 0xf6, 0x57, 0x02, 0xa9, 0xf4, 0x4f, 0x03, 0xa9, 0xfd, 0x7b, 0x04, 0xa9, 0xfd, 0x03, 0x01,
	0x91, 0xf3, 0x03, 0x01, 0xaa, 0xe0, 0x1f, 0x00, 0xb9, 0x28, 0x00, 0x40, 0xf9, 0xe0, 0x03, 0x1f, 0x32, 0xe1, 0x03,
	0x00, 0x32, 0x02, 0x00, 0x80, 0x52, 0x00, 0x01, 0x3f, 0xd6, 0x1f, 0x04, 0x00, 0x31, 0x80, 0x05, 0x00, 0x54, 0xf4,
	...
	0xb2, 0x04, 0x00, 0x00, 0x14, 0xf5, 0x03, 0x46, 0xb2, 0x02, 0x00, 0x00, 0x14, 0x15, 0xa0, 0xe0, 0xd2, 0x68, 0x2a,
	0x40, 0xf9, 0xe0, 0x03, 0x14, 0xaa, 0x00, 0x01, 0x3f, 0xd6, 0xe0, 0x03, 0x15, 0xaa, 0xfd, 0x7b, 0x44, 0xa9, 0xf4,
	0x4f, 0x43, 0xa9, 0xf6, 0x57, 0x42, 0xa9, 0xff, 0x43, 0x01, 0x91, 0xc0, 0x03, 0x5f, 0xd6
};

Rule

The variable name shouldn't be more verbose than its type name.

Wrong

private void schedule_idle (owned ScheduledFunc function) { ... }

Correct

private void schedule_idle (owned ScheduledFunc func) { ... }

Order of functions in a file

Rule

Higher level functions must be placed before lower level functions, and Should be sorted chronologically. In case of C code the corresponding forward declarations must follow the same order.

Wrong

function getInfoAboutA() {
    /* complex code */
    return allInfoAboutA;
}

function getInfoAboutB() {
    /* more complex code */
    return allInfoAboutB;
}

function getInfo() {
    return {
        infoAboutA: getInfoAboutA(),
        infoAboutB: getInfoAboutB()
    };
}

Correct

function getInfo() {
    return {
        infoAboutA: getInfoAboutA(),
        infoAboutB: getInfoAboutB()
    };
}

function getInfoAboutA() {
    /* complex code */
    return allInfoAboutA;
}

function getInfoAboutB() {
    /* more complex code */
    return allInfoAboutB;
}

Rule

For exported functions / public methods: functions that work in all cases come before functions which work only under certain conditions.

Wrong

function getPrivilegedInfo() {
    if (!amIRoot()) {
        throw new Error("You must be root");
    }
    ...
    return privilegedInfo;
}

function getHarmlessInfo() {
    return harmlessInfo;
}

Correct

function getHarmlessInfo() {
    return harmlessInfo;
}

function getPrivilegedInfo() {
    if (!amIRoot()) {
        throw new Error("You must be root");
    }
    ...
    return privilegedInfo;
}

Don't repeat yourself

Rule

Instead of repeating chunks of code, extract it to a function and call it multiple times.

Example

There's no fixed recipe to define what a "repeated chunk of code is", it really depends on the context. For example, even a couple of lines may be worth refactoring into a function, especially if they require some fairly complex error-handling logic.

private async LLDB.Client start_lldb_service (Fruity.LockdownClient lockdown, Cancellable? cancellable)
		throws Error, LLDB.Error, IOError {
	try {
		var lldb_stream = yield lockdown.start_service (DEBUGSERVER_SERVICE_NAME + "?tls=handshake-only", cancellable);
		return yield LLDB.Client.open (lldb_stream, cancellable);
	} catch (Fruity.LockdownError e) {
		if (e is Fruity.LockdownError.INVALID_SERVICE) {
			throw new Error.NOT_SUPPORTED ("This feature requires an iOS Developer Disk Image to be mounted; " +
				"run Xcode briefly or use ideviceimagemounter to mount one manually");
		}
		throw new Error.NOT_SUPPORTED ("%s", e.message);
	}
}

Indentation

Rule

Minimize nesting when possible.

Wrong

function doSomethingMaybe() {
    if (condition) {
        /* do something here */
    }
}

Correct

function doSomethingMaybe() {
    if (!condition) {
        return;
    }
    /* do something here */
}

Rule

Hanging indent – when breaking a long line of code – should be twice the regular indent, regardless of the indentation rules.

Wrong

recursive_init_address = gum_darwin_module_resolve_symbol_address (dyld,
  "__ZN11ImageLoader23recursiveInitializationERKNS_11LinkContextEjRNS_21InitializerTimingListERNS_15UninitedUpwardsE");

Correct

recursive_init_address = gum_darwin_module_resolve_symbol_address (dyld,
    "__ZN11ImageLoader23recursiveInitializationERKNS_11LinkContextEjRNS_21InitializerTimingListERNS_15UninitedUpwardsE");

Spaces

Rule

No trailing spaces.

Indexed loops

Rule

Prefer strict inequality for loop conditions.

Wrong

for (let i = 0; i < len; i++) {
  ...
}

Correct

for (let i = 0; i !== len; i++) {
  ...
}

Comparisons

Rule

Use explicit comparisons, even with 0, NULL, and undefined, instead of relying on language-specific coercion. Only use implicit for booleans.

Wrong

if (value) {
  ...
}

Correct

if (value !== 0) {
  ...
}

Argument validation

Rule

Internal APIs should assume the API contract is not violated – i.e. that the function is passed everything it needs – and omit any check on the arguments.

Wrong

function doSomethingInternallyWithDevice(device) {
  if (device === null || device === undefined) {
     throw new Error('Pass a valid device');
  }
  
  /* do the thing with device */
}

Correct

function doSomethingInternallyWithDevice(device) {
  /* do the thing with device */
}

Rules for C

This set of rules applies to C code, regardless of the project owning it.

File structure

Rule

Every C file must follow the following structure, in this order:

  • Own header includes
  • Internal header includes
  • System/dependency includes
  • typedefs
  • enum and struct definitions
  • Forward declarations / prototypes
  • Global state / static variables
  • Implementation code

Each such group should be separated by a blank line, and includes should be listed in alphabetical order.

Spaces

Rule

In function calls and function definitions, put a space before the parenthesis.

Wrong

ret = thread_create(task, &instance->thread);

Correct

ret = thread_create (task, &instance->thread);

Rule

When declaring pointers, put a space before and after *.

Wrong

GumModuleDetails *details;

Correct

GumModuleDetails * details;

Rule

When casting pointers, put a space before *.

Wrong

init_func = (guint32*) gum_darwin_read (task, addr, sizeof (guint32), NULL);

Correct

init_func = (guint32 *) gum_darwin_read (task, addr, sizeof (guint32), NULL);

Rule

Space before and after binary operators, no spaces around unary operators.

Wrong

for (port_index=0; port_index!=previous_ports->count; port_index ++)
{
  ...
}

* count ++;

Correct

for (port_index = 0; port_index != previous_ports->count; port_index++)
{
  ...
}

*count++;

Rule

Never use more than one space (when not used for indentation).

Wrong

gint  i;

Correct

gint i;

Blank Lines

Rule

It's encouraged to leave one blank line when needing to separate semantically distinct blocks of code or improve readability (some examples later). Just don't abuse that and never leave more than one blank line. (Except for Python code, which should follow the recommendations in PEP-8.)

Wrong

static void
function_one (void)
{
  ...
}


static void
function_two (void)
{
  ...
}

Correct

static void
function_one (void)
{
  ...
}

static void
function_two (void)
{
  ...
}

Function definitions

Rule

Function names must be lowercase, e.g. find_libsystem NOTE: this is true also for arguments, variable names, and labels.

Wrong

static gboolean
frida_find_libSystem (const GumModuleDetails * details, gpointer user_data)
{
    ...
}

Correct

static gboolean
frida_find_libsystem (const GumModuleDetails * details, gpointer user_data)
{
    ...
}

Rule

Functions which take no arguments should be declared (void), as () means that no information about the number or types of the arguments is supplied. (Unlike C++, where () means "no arguments".)

Wrong

static gboolean 
function_without_args ()
{
  ...
}

Correct

static gboolean 
function_without_args (void)
{
  ...
}

Local variables

Rule

Local variables are all declared at the beginning of the block they're used in, a blank line is usually left right after.

Wrong

  if (is_uninitialized_clone)
  {
    mach_port_mod_refs (self_task, task, MACH_PORT_RIGHT_SEND, 1);
    instance->task = task;

    mach_vm_address_t data_address = instance->remote_agent_context;
    ...
  }

Correct

  if (is_uninitialized_clone)
  {
    mach_vm_address_t data_address;

    mach_port_mod_refs (self_task, task, MACH_PORT_RIGHT_SEND, 1);
    instance->task = task;

    data_address = instance->remote_agent_context;
    ...
  }

Rule

Local variables are listed in chronological order of usage.

Exception 1: the variable which holds the return value of a function should be the first in the list, preceded by any variable holding argument values.

Exception 2: related variables of the same type can be grouped in one line, in relative chronological order.

Example 1

Variables in chronological order of usage.

    case FRIDA_BREAKPOINT_CLEANUP:
    {
      task_t self_task;
      gsize page_size;
      FridaExceptionPortSet * previous_ports;
      mach_msg_type_number_t port_index;
      guint i;

      self_task = mach_task_self ();
      page_size = getpagesize ();

      previous_ports = &self->previous_ports;
      for (port_index = 0; port_index != previous_ports->count; port_index++)
      {
        ...
      }
      
      ...

      for (i = 0; i != FRIDA_MAX_BREAKPOINTS; i++)
        frida_spawn_instance_unset_nth_breakpoint (self, i);
    }

Example 2

The variable holding the return value comes first.

static csh
frida_create_capstone (GumCpuType cpu_type, GumAddress start)
{
  csh capstone;
  cs_err err;

  switch (cpu_type)
  {
    case GUM_CPU_ARM64:
      err = cs_open (CS_ARCH_ARM64, CS_MODE_LITTLE_ENDIAN, &capstone);
      break;
    ...
  }

  g_assert (err == CS_ERR_OK);

  return capstone;
}

Example 3

Argument helper variable before return value.

guint
_frida_darwin_helper_backend_demonitor_and_clone_injectee_state (FridaDarwinHelperBackend * self, void * raw_instance)
{
  FridaInjectInstance * instance = raw_instance;
  FridaInjectInstance * clone;

  ...

  return clone->id;
}

Rule

Avoid redundant initialization.

Wrong

cs_insn * insn = NULL;

...

insn = cs_malloc (capstone);

Correct

cs_insn * insn;

...

insn = cs_malloc (capstone);

Indentation and {}

Rule

In function definitions, the return type goes on its own line (not for the prototype).

Wrong

static gboolean frida_find_libsystem (const GumModuleDetails * details, gpointer user_data)
{
  ...
}

Correct

static gboolean
frida_find_libsystem (const GumModuleDetails * details, gpointer user_data)
{
  ...
}

Rule

Curly braces go on a new line, in all cases.

Wrong

if (cached_address_for_breakpoint[instance->cpu_type] == 0) {
  ...
}

Correct

if (cached_address_for_breakpoint[instance->cpu_type] == 0) 
{
  ...
}

Rule

Code in a block is indented by 2 spaces.

Wrong

if (magic == NULL)
    goto handle_probe_dyld_error;

Correct

if (magic == NULL)
  goto handle_probe_dyld_error;

Rule

Simple if statements in which the code is only one line should not have curly braces.

Wrong

if (magic == NULL)
{
  goto handle_probe_dyld_error;
}

Correct

if (magic == NULL)
  goto handle_probe_dyld_error;

Rule

If the if statement has curly braces, then also the else should have it (and vice-versa).

Wrong

    if (error == nil)
      [service cleanupClientPort:client_port];
    else
    {
      g_clear_object (&pipes);

      frida_error = g_error_new (
          FRIDA_ERROR,
          FRIDA_ERROR_NOT_SUPPORTED,
          "Unable to launch iOS app: %s",
          [[error localizedDescription] UTF8String]);
    }

Correct

    if (error == nil)
    {
      [service cleanupClientPort:client_port];
    }
    else
    {
      g_clear_object (&pipes);

      frida_error = g_error_new (
          FRIDA_ERROR,
          FRIDA_ERROR_NOT_SUPPORTED,
          "Unable to launch iOS app: %s",
          [[error localizedDescription] UTF8String]);
    }

Rule

When an if statement has single-statement bodies but any of them exceed the maximum line length, curly braces must be used.

Wrong

  if (_frida_get_springboard_api ()->fbs != NULL)
    frida_darwin_helper_backend_launch_using_fbs (identifier_value, url_value, options, aux_options, on_complete,
        on_complete_target);
  else
    frida_darwin_helper_backend_launch_using_sbs (identifier_value, url_value, options, aux_options, on_complete,
        on_complete_target);

Correct

  if (_frida_get_springboard_api ()->fbs != NULL)
  {
    frida_darwin_helper_backend_launch_using_fbs (identifier_value, url_value, options, aux_options, on_complete,
        on_complete_target);
  }
  else
  {
    frida_darwin_helper_backend_launch_using_sbs (identifier_value, url_value, options, aux_options, on_complete,
        on_complete_target);
  }

Rule

If the condition of an if statement has been broken up into multiple lines, then use braces regardless.

Wrong

    if ((ctx->sink_mask & GUM_BLOCK) != 0 &&
        gum_x86_relocator_eob (rl) &&
        insn.ci->id != X86_INS_CALL)
      gum_exec_block_write_block_event_code (block, &gc, GUM_CODE_INTERRUPTIBLE);

Correct

    if ((ctx->sink_mask & GUM_BLOCK) != 0 &&
        gum_x86_relocator_eob (rl) &&
        insn.ci->id != X86_INS_CALL)
    {
      gum_exec_block_write_block_event_code (block, &gc, GUM_CODE_INTERRUPTIBLE);
    }

Switch statements

Rule

If a case needs curly braces, the break goes inside, usually preceded by a blank line.

Wrong

switch (cpu_type)
{
  case GUM_CPU_ARM:
  {
    /* case implementation*/
  }
  break;
  
  ...
}

Correct

switch (cpu_type)
{
  case GUM_CPU_ARM:
  {
    /* case implementation*/
    
    break;
  }
  
  ...
}

Rules for JavaScript

This set of rules apply to JavaScript code, regardless of the project owning it.

String constants

Rule

Don't use double quotes.

Wrong

throw new Error("Invalid argument");

Correct

throw new Error('Invalid argument');

Indentation

Rule

Indent with 2 spaces.

Rule

Curly braces go on the same line of the statement.

Wrong

if (condition)
{
  ...
}
else
{
  ...
}

Correct

if (condition) {
  ...
} else {
  ...
}

Semicolons

Rule

Treat semicolons as mandatory.

Wrong

console.log('hello world')

Correct

console.log('hello world');

Comparisons

Rule

Use strict comparisons.

Wrong

if (methodName == '- init') {
  ...
}

Correct

if (methodName === '- init') {
  ...
}

Rule

Put parenthesis around the ternary comparison condition unless it's simply referencing a boolean variable.

Wrong

m = res[0] === '' ? '*' : res[0];

Correct

m = (res[0] === '') ? '*' : res[0];

Spaces

Rule

Unless the specific project follows the semistandard conventions, put no spaces between function name and argument list.

Wrong

function parseExportsFunctionPattern (pattern) {
  var res = pattern.split ('!');
  ...
}

Correct

function parseExportsFunctionPattern(pattern) {
  var res = pattern.split('!');
  ...
}

Object properties

Rule

Reference object properties without quotes when possible.

Wrong

enumerateMatches('exports:' + obj['module'] + '!' + obj['function']);

Correct

enumerateMatches('exports:' + obj.module + '!' + obj.function);

Internal agents

Rule

Stick to ES5 syntax, so it can be consumed by the Duktape runtime without having to first frida-compile the code.

Rule

In rpc.exports, dispose() comes first, or right after init().

Rules for TypeScript

This set of rules apply to TypeScript code, regardless of the project owning it. Rules for JavaScript also apply to TypeScript, if not explicitly overridden.

Indentation

Rule

Indent with 4 spaces.

Spaces

Rule

Never put spaces between function name and argument list, neither in calls nor definitions.

String constants

Rule

Don't use single quotes.

Enums

Rule

The TypeScript convention is pascal-case for enum values, e.g. FooBarBaz.

Wrong

enum PlistType {
    NONE,
    BINARY,
    XML
}

Correct

enum PlistType {
    None,
    Binary,
    Xml
}

Rule

Usage of const enum is discouraged portability-wise.

Wrong

export const enum GrassColor {
    Yellow = "YELLOW",
    LightGreen = "LIGHTGREEN",
    Green = "GREEN",
    DarkGreen = "DARKGREEN"
}

Correct

export type GrassColor = 
    | "YELLOW" 
    | "LIGHTGREEN" 
    | "GREEN" 
    | "DARKGREEN"
    ;

Constants

Rule

Constants should be uppercase, e.g. FOO_BAR_BAZ.

Wrong

const lockdownPort = 62078;

Correct

const LOCKDOWN_PORT = 62078;

Types

Rule

Use interface when possible.

Wrong

export type StringDict = {
    [name: string]: string;
}

Correct

export interface StringDict {
    [name: string]: string;
}

Rules for Vala

This set of rules apply to Vala code, regardless of the project owning it.

Order of class properties

Rule

  • Public properties should precede private ones.
  • Higher level properties should precede lower level ones.

Indentation, spaces and {}

Rule

Indent only with tabs.

Rule

Rules for spaces are the same as C.

Rule

Curly braces go on the same line of the statement. Follow the same rules as C for whether to omit them or not.

Wrong

if (condition)
{
  ...
}
else
{
  ...
}

Correct

if (condition) {
  ...
} else {
  ...
}

Usage of var type

Rule

Declare variables as var, especially when the type is obvious.

Wrong

Json.Node parameters = new Json.Node (Json.NodeType.OBJECT);

Correct

var parameters = new Json.Node (Json.NodeType.OBJECT);

Generic rules for Python

This set of rules apply to all python code, regardless of the project it belongs to.

Formatting

Rule

Python formatting should follow the PEP-8 guidelines.

String quotes

Rule

Use double quotes for regular strings, single quotes for enum-like values.

Wrong

if os.environ.get("TERM", '') == "dumb":

Correct

if os.environ.get("TERM", "") == 'dumb':

Imports

Rule

Imports go in alphabetical order.

Comments

Rule

Comments should use a capital letter at the start of each sentence, and each should end in a full stop.

Generic rules in frida-gum

This set of rules apply to all code in frida-gum, regardless of the language.

Max line length

Rule

Lines should not exceed 80 characters.

Rules for C code in frida-gum

Function definitions

Rule

When defining a function implementation each argument must go on its own line, vertically aligned (not for the prototype).

Wrong

static void
gum_exec_block_write_block_event_code (GumExecBlock * block, GumGeneratorContext * gc,
                                       GumCodeContext cc)
{
  ...
}

Correct

static void
gum_exec_block_write_block_event_code (GumExecBlock * block,
                                       GumGeneratorContext * gc,
                                       GumCodeContext cc)
{
  ...
}

Generic rules in frida-core

This set of rules apply to all code in frida-core, regardless of the language.

Max line length

Rule

Lines should not exceed 140 characters.

Rules for C code in frida-core

This set of rules apply to C code belonging to frida-core.

Function definitions

Rule

Function names should be “namespaced” by having a frida_ prefix, even if static.

Wrong

static gboolean
find_libsystem (const GumModuleDetails * details, gpointer user_data)
{
  ...
}

Correct

static gboolean
frida_find_libsystem (const GumModuleDetails * details, gpointer user_data)
{
  ...
}