diff options
author | Kristján Valur <kristjan@rvx.is> | 2019-02-27 16:32:14 +0000 |
---|---|---|
committer | Noel Power <npower@samba.org> | 2019-03-07 14:08:22 +0000 |
commit | 1ff252e398c6dc140570821394a7cda262af6dd9 (patch) | |
tree | e8922fa32385170a42d3f7faf37b5905a5343069 /libgpo | |
parent | a8b316d102e0e864dde4ab39cc20b98f32216ff4 (diff) | |
download | samba-1ff252e398c6dc140570821394a7cda262af6dd9.tar.gz |
pygpo: More python exception cleanup.
* Don't override existing exceptions.
* Careful with talloc contexts.
* Return NULL on error.
* Add more information to exception messages from internal functions.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13822
Signed-off-by: Kristján Valur Jónsson <kristjan@rvx.is>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
Diffstat (limited to 'libgpo')
-rw-r--r-- | libgpo/pygpo.c | 103 |
1 files changed, 51 insertions, 52 deletions
diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c index 7ae3de31973..5e012dfc972 100644 --- a/libgpo/pygpo.c +++ b/libgpo/pygpo.c @@ -157,6 +157,7 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds) PyObject *py_creds = NULL; PyObject *lp_obj = NULL; struct loadparm_context *lp_ctx = NULL; + bool ok = false; static const char *kwlist[] = { "ldap_server", "loadparm_context", "credentials", NULL @@ -168,34 +169,28 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds) } if (py_creds) { - if (!py_check_dcerpc_type(py_creds, "samba.credentials", - "Credentials")) { - PyErr_Format(PyExc_TypeError, - "Expected samba.credentials " - "for credentials argument"); + ok = py_check_dcerpc_type(py_creds, "samba.credentials", + "Credentials"); + if (!ok) { return -1; } self->cli_creds = PyCredentials_AsCliCredentials(py_creds); } - if (lp_obj) { - bool ok = py_check_dcerpc_type(lp_obj, "samba.param", - "LoadParm"); - if (!ok) { - PyErr_Format(PyExc_TypeError, - "Expected samba.param.LoadParm " - "for lp argument"); - return -1; - } - lp_ctx = pytalloc_get_type(lp_obj, struct loadparm_context); - if (lp_ctx == NULL) { - return -1; - } - ok = lp_load_initial_only(lp_ctx->szConfigFile); - if (!ok) { - return -1; - } + ok = py_check_dcerpc_type(lp_obj, "samba.param", "LoadParm"); + if (!ok) { + return -1; + } + lp_ctx = pytalloc_get_type(lp_obj, struct loadparm_context); + if (lp_ctx == NULL) { + return -1; + } + ok = lp_load_initial_only(lp_ctx->szConfigFile); + if (!ok) { + PyErr_Format(PyExc_RuntimeError, "Could not load config file '%s'", + lp_ctx->szConfigFile); + return -1; } if (self->cli_creds) { @@ -206,15 +201,9 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds) workgroup = lp_workgroup(); } - if (ldap_server == NULL) { - return -1; - } - + /* always succeeds or crashes */ self->ads_ptr = ads_init(realm, workgroup, ldap_server); - if (self->ads_ptr == NULL) { - return -1; - } - + return 0; } @@ -305,11 +294,13 @@ static PyObject *py_gpo_get_sysvol_gpt_version(PyObject * self, PyObject *result; NTSTATUS status; - tmp_ctx = talloc_new(NULL); - if (!PyArg_ParseTuple(args, "s", &unix_path)) { return NULL; } + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return PyErr_NoMemory(); + } status = gpo_get_sysvol_gpt_version(tmp_ctx, unix_path, &sysvol_version, &display_name); @@ -319,8 +310,8 @@ static PyObject *py_gpo_get_sysvol_gpt_version(PyObject * self, return NULL; } - talloc_free(tmp_ctx); result = Py_BuildValue("[s,i]", display_name, sysvol_version); + talloc_free(tmp_ctx); return result; } @@ -394,8 +385,8 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) uint32_t uac = 0; uint32_t flags = 0; struct security_token *token = NULL; - PyObject *ret = Py_None; - TALLOC_CTX *gpo_ctx; + PyObject *ret = NULL; + TALLOC_CTX *gpo_ctx = NULL; size_t list_size; size_t i; @@ -403,10 +394,7 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) if (!PyArg_ParseTupleAndKeywords(args, kwds, "s", discard_const_p(char *, kwlist), &samaccountname)) { - PyErr_SetString(PyExc_RuntimeError, - "Failed to parse arguments to " - "py_ads_get_gpo_list()"); - goto out; + return NULL; } frame = talloc_stackframe(); @@ -414,9 +402,9 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) status = find_samaccount(self->ads_ptr, frame, samaccountname, &uac, &dn); if (!ADS_ERR_OK(status)) { - TALLOC_FREE(frame); - PyErr_SetString(PyExc_RuntimeError, - "Failed to find samAccountName"); + PyErr_Format(PyExc_RuntimeError, + "Failed to find samAccountName '%s': %s", + samaccountname, ads_errstr(status)); goto out; } @@ -425,21 +413,33 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) flags |= GPO_LIST_FLAG_MACHINE; status = gp_get_machine_token(self->ads_ptr, frame, dn, &token); + if (!ADS_ERR_OK(status)) { + PyErr_Format(PyExc_RuntimeError, + "Failed to get machine token for '%s'(%s): %s", + samaccountname, dn, ads_errstr(status)); + goto out; + } } else { status = ads_get_sid_token(self->ads_ptr, frame, dn, &token); - } - if (!ADS_ERR_OK(status)) { - TALLOC_FREE(frame); - PyErr_SetString(PyExc_RuntimeError, "Failed to get token"); - goto out; + if (!ADS_ERR_OK(status)) { + PyErr_Format(PyExc_RuntimeError, + "Failed to get sid token for '%s'(%s): %s", + samaccountname, dn, ads_errstr(status)); + goto out; + } } gpo_ctx = talloc_new(frame); + if (!gpo_ctx) { + PyErr_NoMemory(); + goto out; + } status = ads_get_gpo_list(self->ads_ptr, gpo_ctx, dn, flags, token, &gpo_list); if (!ADS_ERR_OK(status)) { - TALLOC_FREE(frame); - PyErr_SetString(PyExc_RuntimeError, "Failed to fetch GPO list"); + PyErr_Format(PyExc_RuntimeError, + "Failed to fetch GPO list: %s", + ads_errstr(status)); goto out; } @@ -452,7 +452,6 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) i = 0; ret = PyList_New(list_size); if (ret == NULL) { - TALLOC_FREE(frame); goto out; } @@ -460,7 +459,7 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) PyObject *obj = pytalloc_reference_ex(&GPOType, gpo_ctx, gpo); if (obj == NULL) { - TALLOC_FREE(frame); + Py_CLEAR(ret); goto out; } @@ -469,7 +468,7 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds) } out: - + TALLOC_FREE(gpo_ctx); TALLOC_FREE(frame); return ret; } |