[PATCH 1 of 2] phase: compute phases in C

Matt Mackall mpm at selenic.com
Fri Mar 20 17:28:55 CDT 2015


On Fri, 2015-03-20 at 14:08 -0700, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1426874708 25200
> #      Fri Mar 20 11:05:08 2015 -0700
> # Node ID 1c9600e6ceab57b48827292c7b01ed9a625da1fd
> # Parent  b7f936f47f2b104a60840bae571e009742126afc
> phase: compute phases in C

>  #include "util.h"
> +#define MIN(a, b) (((a)<(b))?(a):(b))

If we want to have a line like the second line anywhere, it's probably
inside the firstline. 

> +	/* Build list of markers to avoid creating python objects repeatedly */
> +	phasemarkers = calloc(numphase, sizeof(PyObject *));

> +	if (phasemarkers == NULL) {
> +		goto bail;
> +	}
> +	for (i = 0; i < numphase; i++) {
> +		phasemarkers[i] = PyInt_FromLong(i);
> +	}
> +
> +	/* Transform phase list to a python list */
> +	PyObject *phaseslist = PyList_New(len);
> +	Py_INCREF(phaseslist);
> +	if (phaseslist == NULL) {
> +		Py_XDECREF(phaseslist);
> +		goto bail;
> +	}
> +	for (i = 0; i < len; i++) {
> +		PyList_SET_ITEM(phaseslist, i, phasemarkers[(int)phases[i]]);
> +	}
> +
> +	/* Cleanup and return */
> +	free(phases);
> +	for (i = 0; i < numphase; i++) {
> +		Py_XDECREF(phasemarkers[i]);
> +	}
> +	free(phasemarkers);
> +	return phaseslist;
> +
> +bail:
> +	if (phases != NULL) {
> +		free(phases);
> +	}
> +	return Py_None;

This is not quite right. This should illustrate the goto error-handling
pattern a bit more clearly:

int func(void)
{
	int ret = -1; /* default error */
	char *a, *b, *c;
	
	a = malloc();
	if (!a)
		goto release_none;
	...
	if (some_error) {
		ret = -2; /* special error code */
		goto release_a;
	...
	b = malloc();
	if (!b)
		goto release_a;
	...
	if (something()) {
		...
		c = malloc();
		if (!c)
			goto release_b;		
		...
		while(something_else()) {
			...
			if (we_need_to_exit_early)
				goto release_c;
			...
			}
		}
	}
	...
	ret = our_result;

release_c:
	free(c);
release_b:
	free(b);
release_a:
	free(a);
release_none:
	return ret;
}

In short:

- one exit point
- one allocation / one free per resource
- release resources in reverse order
- code statically knows what needs releasing

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list