As the title suggests, I'm trying to create a bunch of attributes but the code is getting repetitive and messy. I want to use the closure argument to make the code more compact.

According to the C API reference, the closure is a function pointer that provides additional information for getters/setters. I have not been able to find an example of it in use.

This is how I am currently using it:

static void closure_1() {};
static void closure_2() {};
...
static PyObject *
FOO_getter(FOO* self, void *closure) {
    if (closure == &closure_1) {
        return self->bar_1;
    } else if (closure == &closure_2) {
        return self->bar_2;
    }
}
static int
FOO_setter(FOO* self, PyObject *value, void *closure) {
    if (closure == &closure_1) {
        if (somehow value is invalid) {
            PyErr_SetString(PyExc_ValueError, "invalid value for bar_1.");
            return -1;
        }
    } else if (closure == closure_2) {
        if (somehow value is invalid) {
            PyErr_SetString(PyExc_ValueError, "invalid value for bar_2.");
            return -1;
        }
    }
    return 0;
}
static PyGetSetDef FOO_getsetters[] = {
    {"bar_1", (getter) FOO_getter, (setter) FOO_setter, "bar_1 attribute", closure_1},
    {"bar_2", (getter) FOO_getter, (setter) FOO_setter, "bar_2 attribute", closure_2},
    {NULL}  /* Sentinel */
};
...

It works the way I want it to, but it looks more like a hack than something "pythonic". Is there a better way to handle this? e.g. call the closure in some way.

CodePudding user response：

I guess this "closure" is used to pass an extra context to Foo_getter. It should be something that simplifies accessing members of Foo. Documentation it likely wrong. It should be "optional pointer", not "optional function pointer".

Consider passing offsets of the members. Offsets to struct members can be easily obtained with standard offsetof macro defined in stddef.h. It is a small unsigned integer that will fit to void* type.

static PyGetSetDef FOO_getsetters[] = {
    {"bar_1", (getter) FOO_getter, (setter) FOO_setter, "bar_1 attribute", (void*)offsetof(FOO, bar_1)},
    {"bar_2", (getter) FOO_getter, (setter) FOO_setter, "bar_2 attribute", (void*)offsetof(FOO, bar_2)},
    {NULL}  /* Sentinel */
};

Now the getter could be:

static PyObject *
FOO_getter(FOO* self, void *closure) {
    // pointer to location where the FOO's member is stored
    char *memb_ptr = (char*)self   (size_t)closure;
    // cast to `PyObject**` because `mem_ptr` points to location where a pointer to `PyObject` is stored
    return *(PyObject**)mem_ptr;
}

Use similar schema for the setter.

CodePudding user response：

Despite the documentation, I'm assuming the closure can be any pointer you want. So how about passing an "object", seeing as C doesn't support closures (short of literally generating functions at run-time).

In an object, we can store the offset of the member in FOO, and a pointer to an attribute-specific validator.

typedef int (*Validator)(FOO *, const struct Attribute *, void *);

typedef struct Attribute {
   const char *name;
   size_t offset;
   Validator validator;
} Attribute;

static PyObject **resolve_offset(FOO *self, const Attribute *attr) {
   return (PyObject **)( ( (char *)self )   attr->offset );
}

static PyObject *FOO_getter(FOO *self, void *_attr) {
   const Attribute *attr = (const Attribute *)_attr;
   return *resolve_offset(self, attr);
}

static int FOO_setter(FOO *self, PyObject *val, void *_attr) {
   const Attribute *attr = (const Attribute *)_attr;
   if (attr->validator(self, attr, val)) {
      *resolve_offset(self, attr) = val;
      return 0;
   } else {
      // Building the string to include attr->name is left to you.
      PyErr_SetString(PyExc_ValueError, "invalid value.");
      return -1;
   }
}

static int FOO_bar_1_validator(FOO *self, const Attribute *attr, void *val) { ... }
static int FOO_bar_2_validator(FOO *self, const Attribute *attr, void *val) { ... }

#define ATTRIBUTE(name)                      \
   static Attribute FOO_ ## name ## attr = { \
      #name,                                 \
      offsetof(FOO, name),                   \
      FOO_ ## name ## _validator             \
   };

ATTRIBUTE(bar_1);
ATTRIBUTE(bar_2);

#define PY_ATTR_DEF(name) { \
   #name,                   \
   (getter)FOO_getter,      \
   (setter)FOO_setter,      \
   #name " attribute",      \
   &(FOO_ ## name ## attr)  \
}

static PyGetSetDef FOO_getsetters[] = {
    PY_ATTR_DEF(bar_1),
    PY_ATTR_DEF(bar_2),
    { NULL }
};

I originally wrote:

resolve_offset surely relies on undefined behaviour, but it should work fine. The alternative would be to have three functions in our attribute object (get, validate, set) instead of one, but that defies the point of the question.

But @tstanisl points out that it looks like it isn't UB. Awesome!