Hi,
most of the python bindings code looks ok, but I think that you shouldn't pass native binary objects to other objects directly, but use the python way instead. Some people may want to extend your class in some way and we shouldn't be making more obstacles than necessary.
I'm talking about the code here:
> static PyObject *PyIscsiNode_setAuth(PyObject *self, PyObject *args,
> PyObject *kwds)
> {
> char *kwlist[] = {"authinfo", NULL};
> int err;
> PyIscsiNode *node = (PyIscsiNode *)self;
> PyObject *arg;
> const struct libiscsi_auth_info *authinfo = NULL;
>
> if (!PyArg_ParseTupleAndKeywords(args, kwds, "O", kwlist, &arg))
> return NULL;
>
> if (arg == Py_None) {
> authinfo = NULL;
> } else if (PyObject_IsInstance(arg, (PyObject *)
> &PyIscsiChapAuthInfo_Type)) {
> PyIscsiChapAuthInfo *pyauthinfo = (PyIscsiChapAuthInfo *)arg;
Especially about this line
> authinfo = &pyauthinfo->info;
In my oppinion you should create local authinfo structure and fill it with values by using something like:
const struct libiscsi_auth_info authinfo;
memset(&authinfo, 0, sizeof(struct libiscsi_auth_info));
authinfo.method = libiscsi_auth_chap;
/* get all the information we need from IscsiChapAuthInfo */
PyObject* p_username = PyObject_GetAttrString(arg, "username");
PyObject* p_rusername = PyObject_GetAttrString(arg, "reverse_username");
PyObject* p_password = PyObject_GetAttrString(arg, "password");
PyObject* p_rpassword = PyObject_GetAttrString(arg, "reverse_password");
/* convert to C, so we can use libiscsi */
if(!(p_username && p_rusername && p_password && p_rpassword)){
PyErr_SetString(PyExc_ValueError, "invalid authinfo type");
return NULL;
}
char *su, *sru, *sp, *srp;
int ok = PyArg_ParseTuple(p_username, "s", &su);
ok = ok && PyArg_ParseTuple(p_rusername, "s", &sru);
ok = ok && PyArg_ParseTuple(p_password, "s", &sp);
ok = ok && PyArg_ParseTuple(p_rpassword, "s", &srp);
if(ok){
strcpy(authinfo.chap.username, su);
strcpy(authinfo.chap.reverse_username, sru);
strcpy(authinfo.chap.password, sp);
strcpy(authinfo.chap.reverse_password, srp);
err = libiscsi_node_set_auth(context, &node->node, &authinfo);
if (err) {
PyErr_SetString(PyExc_IOError,
libiscsi_get_error_string(context));
}
}
else{
PyErr_SetString(PyExc_ValueError, "invalid authinfo type");
}
/* decrement counters */
Py_XDECREF(p_username);
Py_XDECREF(p_rusername);
Py_XDECREF(p_password);
Py_XDECREF(p_rpassword);
this way it would support correct subclassing of the PyIscsiChapAuthInfo.
It is more code, a little bit slower, but it is the correct python way of passing arguments between classes. Were there a method returning all the auth info in one tuple/dictionary, the code would be simplier than this.
> Py_RETURN_NONE;
> }
>
Martin
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
