On Thursday, June 16, 2011 22:12:36 steve tell wrote: > - code formatting much closer to the style of the rest of urjtag (but > claiming perfection)
i still see tabs in the C files ... those should all be spaces
> --- /dev/null
> +++ b/urjtag/bindings/Makefile.am
> +SUBDIRS = $(PYTHON_SUBDIR)
> +
> +if BIND_PYTHON
> +PYTHON_SUBDIR=python
> +else
> +PYTHON_SUBDIR=
> +endif
does it work if you use this simpler style ?
SUBDIRS =
if BIND_PYTHON
SUBDIRS += python
endif
> --- /dev/null
> +++ b/urjtag/bindings/python/Makefile.am
> @@ -0,0 +1,36 @@
> +EXTRA_DIST = setup.py chain.c pycompat23.h t_urjtag_chain.py
> +
> +all-local: build
> +
> +build: chain.c
> + -{ $(PYTHON) setup.py build && touch build; } || { $(RM) -r build; exit
> 1; } +
> +install-data-local:
> + -$(PYTHON) setup.py install --prefix=$(DESTDIR)$(prefix)
these shouldnt be ignored when there's a failure (the leading "-")
> --- /dev/null
> +++ b/urjtag/bindings/python/chain.c
> +#include <sys/mman.h>
this isnt portable, and i dont see you actually using mmap() in this file, so
i guess you can just drop it ?
> +/* missing from urjtag headers */
> +extern int urj_cmd_get_number (const char *s, long unsigned *i);
> +extern int urj_cmd_test_cable (urj_chain_t * chain);
then add them to the proper headers. externs in C files are generally frowned
upon.
> +typedef struct {
> + PyObject_HEAD urj_chain_t * urchain;
no space between the "*" and the var name. so it should be "void *foo".
> + //printf ("in Chain_dealloc urc=%p\n", self);
drop the dead code please
> +static PyObject *
> +Chain_new (PyTypeObject * type, PyObject * args, PyObject * kwds)
> +{
> + Chain *self;
> +
> + self = (Chain *) type->tp_alloc (type, 0);
> + if (self != NULL)
for NULL pointer checks, it's generally better to return immediately rather
than put the entire function body inside of an if statement
> --- /dev/null
> +++ b/urjtag/bindings/python/urj_helpers.c
> @@ -0,0 +1,161 @@
> + * Additional urjtag cain operations.
> + * These are not language-binding specific, but do help in writing of
> concise
> + * language bindings. In theory, this code could someday be merged into
> + * the core urjtag library.
let's do that now rather than later
> --- a/urjtag/configure.ac
> +++ b/urjtag/configure.ac
> +AC_ARG_ENABLE([python],
> + [AS_HELP_STRING([--enable-python], [build python language bindings for
> liburjtag])],
> + [BIND_PYTHON="yes"],[BIND_PYTHON="no"])
the 3rd arg should be BIND_PYTHON=$enableval
> +if test "$BIND_PYTHON" == "yes"; then
use "=", not "=="
> + if (( $PYTHON_MAJOR_VERSION < 2 )); then
if test "$PYTHON_MAJOR_VERSION" -lt 2 ; then
> +if test "$BIND_PYTHON" == "yes"; then
> +AM_CONDITIONAL([BIND_PYTHON], [test "$BIND_PYTHON" == "yes"])
=, not ==
-mike
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------ EditLive Enterprise is the world's most technically advanced content authoring tool. Experience the power of Track Changes, Inline Image Editing and ensure content is compliant with Accessibility Checking. http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________ UrJTAG-development mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/urjtag-development
