Re: libgit2 - a true git library

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 1, 2008, 02:26 UTC
Message-ID: <alpine.DEB.1.00.0811010320370.22125@pacific.mpi-cbg.de.mpi-cbg.de>
In-Reply-To: <20081101015217.GJ14786@spearce.org>

Hi,

On Fri, 31 Oct 2008, Shawn O. Pearce wrote:

Show 7 quoted lines

> Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> > 
> > > > Both the negative code and errno style are lightweight in the 
> > > > common "no error" case.  The errno style is probably more handy 
> > > > for those functions returning a pointer which should be NULL in 
> > > > the error case.

Unfortunately, errno would not be thread-safe, unless you can guarantee that errno is a thread-local variable.

Show 23 quoted lines

> > > 
> > > I'm sticking with return a negative code for now, to the extent that 
> > > some functions which return a pointer but also have many common 
> > > failure modes (e.g. git_odb_open) use an output parameter as their 
> > > first arg, so the error code can be returned as the result of the 
> > > function.
> > 
> > Actually, the pointer-returning functions can encode error cases into a 
> > "negative" pointer. See include/linux/err.h for example.
> > 
> > 	void *ptr = git_alloc_foo(...);
> > 	if (IS_ERR(ptr))
> > 		die("git_alloc_foo failed: %s", git_strerr(PTR_ERR(ptr)));
> 
> Oh, good point.  We could also stagger the errors so they are
> always odd, and never return an odd-alignment pointer from a
> successful function.  Thus IS_ERR can be written as:
> 
>   #define IS_ERR(ptr) (((intptr_t)(ptr)) & 1)
> 
> which is quite cheap, and given the (probably required anyway)
> aligned allocation policy means we still have 2^31 possible
> error codes available.

Oh boy, both solutions are ugly as hell. Although the &1 method does not limit the memory space as much (except if you plan to work in space-contrained environments, where you do not want to be forced to word-align structs).

The only pointer game I would remotely consider clean is if you had

	const char *errors[] = {
		...
	};

	inline int is_error(void *ptr) {
		return ptr >= errors && ptr < errors + ARRAY_SIZE(errors);
	}

although that would be less performant.

Ciao, Dscho