Hacking mpg123

Here are some minimal guidelines for hacking the mpg123 code, meaning: A description of the coding standards the mpg123 sources follow and that patches to mpg123 should adhere to, to ease integration. Generally, try to get into discussion with the mpg123 team about your patches and ideas on the mpg123-devel mailing list.

There is also a github clone that you can use and pull requests on that one trigger notification to the mailing list, so that there is a chance that the respective change is picked up.

It is C89. Period.

The mpg123 project wants to stay true to its legacy of being the portable MPEG engine UNIX (and Windows, OS/2, ...) folks grew up to love during the end of the last millenium (or the past and current decades, if you prefer;-). One simple rule for that is: All mpg123 source is written in the C language as specified in the ANSI C89 / ISO C90 standard. An exception may be code inside #ifdef specific for a platform that needs non-C89 code for some reason.

Specific hardware optimizations are preferred as stand-alone assembly files, but may be written using intrinsics or other variants if the platform wants it that way.

Extensions to that standard need to be included in a backwards-compatible way.
One example is our usage of types like off_t only after testing for availabilty (in an autoconf macro) and defining it to a substitute type if necessary.
Common system workarounds and inclusion of system-specific headers are collected in the src/compat directory. Code from there is included in builds of mpg123 and friends. A look into that code might give a hint about what kind of stuff is not to be taken for granted on every system mpg123 is built on.
Local variables need to be declared before any other code in a block. That rule might surprise folks, but it really is one when staying true to C89. Note that it only requires you to declare all variables in each block before the code in that block. One function can contain several blocks (if / else, loops, etc.).
C++ / C99 - style comments are not allowed. The only comment marker is /* Comment. */, with variations of multi-linedness and doxygen fun.

A good test for conformity is building the code with a recent gcc and the options -std=c89 -pedantic. There is the configure option --enable-nagging to enable such a build.

Style

I won't discuss much of the reasoning behind coding style here... as it is nonsensical to argue. But one departure from my own personal opinion for many years shall be documented: I caved in and accepted that not formatting long lines with some limit in mind just doesn't work because our tools support us with creating such fixed formatting, but not with automatically creating bearable formatting for display only, keeping logical lines as physical lines. Also line-based diffs are not useful when the lines are too long.

Indentation with literal tab characters, one tab is one level of indent. If you want to do alignment of stuff, use spaces for that, in addition to the indent with the assumption of a fixed-width font. I pondered elastic tab stops since many years ago, but came to the conclusion that it's not worth the trouble, even if the idea seems nice. Even if interspersed with operators (see below), a tab always indicates a level of indent.
Operators should lead a new line instead of trailing the previous one. They may be integrated in indent or alignment to save space and possible for visual enhancement (depending on taste, of course). See example below.
While a single line may be longer if it is impractical to prevent it, the rule for new code is that it should be formatted to stay below the traditional limit of 80 characters. The tab width is a variable there but shall be assumed to be no more than 3. When someone prefers a tab width of 8, that someone must provide horizontal space to match.
Opening braces generally start a new line, so that they line up with the closing brace. If the enclosed block is small enough to stay on one line, opening/closing braces can stay on that line as well. Also, things can be compacted if blocks and code bits before opening brace are small enough. Especially, some code/keyword may follow a closing brace on the same line if it makes sense.
Opening parentheses generally are not separated from functions or keywords, while closing parentheses can be at the end or beginning of a line, depending on the situation. Exceptions are exceptional.
Space right inside parentheses shall be added if the expression spans multiple lines or if it is required to separate nested expressions. So, please do not do that for each simple function call.
In another change from earlier convictions, body code shall always be indented from the parent construct. Braces are optional for single-line blocks. Blank lines may be introduced for clarity, but don't have to.

Example

Here's some example fantasy code.

	/* Leading comma means no change of previous line when appending an entry,
	   comma before tab prevents uglyness because first entry would like
	   alignment with a single space otherwise. */
	enum some_enums
	{
		SOME_CONSTANT
	,	ANOTHER_CONSTANT
	,	YET_ANOTHER_ONE /* No change here when extending the enum. */
	}
	
	/*
		This comment just has too
		many lines for
		me to bother aligning things with
		three spaces to match.
	*/
	
	switch(code)
	{
		case SOME_CONSTANT:
			handle_const1();
		break;
		ANOTHER_CONSTANT:
		{
			int a;
			a = handle_const2();
			printf("%i\n", a);
		}
		break;
		case YET_ANOTHER_ONE:
			printf("Hurray\n");
		break;
		default:
			error("What to do?");
	}
	
	/* This is alignment, not indent, also using a leading
	   comma. */
	int function( int arg, char arg2
	            , int arg2, char arg3 );
	
	/* Here, things just are too long and pretty alignment is no fit. It is
	   preferred to keep function attributes/type on a line with the name. */
	funny_attribute_or_type_macro long function( some_type argument1
	,	another_type argument2, yet_another_type argument3 );
	
	if(TRUE) /* I do not like { at end of line. */
	{
		do_something(a, b); /* Note: No space right inside parentheses. */
		/* Here, things are spacy. */
		do_more( first_argument_is_rather_long
		,	(some_condition)
			?	good_value /* two levels of indent */
			:	bad_value
		);
		a = bleh( /* Again, the ( is not separated from the function name. */
			complicated_argument
		,	function_call( some_stuff
			,	more_stuff, last_stuff )
		,	dunno_something
		); /* But the closing ) may be far away, ending the block visibly. */
	} else
	{
		success = -1;
		do_not_bother;
	} else if(hello_there)
		nudge();
	
	if(yes)
		return yay;
	else
	{
		ask_why();
		return no;
	}
	
	/* You might do pretty formatting if you want to. */
	struct threesome threes[] =
	{
		{ value,  "string value",               3 }
	,	{ valued, "a long litany of tragedy", 123 }
	,	{ v,      "short",                     12 }
	};
	
	/* Things can be combined to avoid indent cascades.
	   Also, this is an example for two-character operators inside indent. */
	for(i=0; i<limit; ++i) if
	(
		!strcasecmp(onething, name)
	||	!strcasecmp(atherthing, name)
	){ /* With only ) before it, the { still is at the beginning of the line */
		do_something();
		return something;
	}

Preprocessing

Well, we have C code, so we do use the C preprocessor. Although one should expect from C89-compliant preprocessors that they can take whitespace before the # introducing some preprocessor directive, it is preferred in mpg123 to put the # really right at the beginning of the line. If indendation of preprocessing directives is necessary (nested #if), then that indent shall be applied directly after the #. This is not (yet) consistent accross mpg123... Oh, and we probably should use tabs in preprocessor indendation just like in code indendation, unless someone gives me a pointer about preprocessors that choke on tabs there.

Documentation

The API online documentation is generated from the public API headers using Doxygen. So, any addition to the official API needs to get some properly formatted comment in the respective header. We do not use Doxygen for the rest of the code, so it is not needed to document internal functions that way.