Stephen Gallagher wrote:
> On 05/06/2010 02:48 PM, Stephen Gallagher wrote:
>>>>> 1) The code seems to be duplicated multiple times. Why common code is
>>>>> not segmented into a common function?
>>>>
>>>> It is now :)
>
> Dmitri pointed out to me off-list that there was a much larger section
> of redundant code I could turn into a common function.
>
> New patch attached.
>
Sorry but let me be a bit picky
+ enum account_restart *err* = SDAP_ACCOUNT_DO_NOT_RESTART;
+
+ if (ret != EOK) {
+ *dp_err = DP_ERR_FATAL;
+
+ if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
+ *dp_err = DP_ERR_OFFLINE;
+ ctx =
talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
+ struct sdap_id_ctx);
+ if (sdap_check_gssapi_reconnect(ctx)) {
+ if (ctx->gsh) {
+ /* Mark the connection as false so we don't try to
use an
+ * invalid connection by mistake later.
+ * If the global sdap handler is NULL, it's ok not
to do
+ * anything here. It's always checked by
sdap_connected()
+ * before being used.
+ */
+ ctx->gsh->connected = false;
+ }
+ *err* = sdap_account_info_restart(breq);
+ if (*err* == *EOK*) return SDAP_ACCOUNT_DO_RESTART;
how about the following approach:
+void sdap_account_info_common_done(int ret, struct be_req *breq, const
char *errstr)
+{
+ struct sdap_id_ctx *ctx;
+ int err = DP_ERR_OK;
+ int error = EOK;
+
+ if (ret != EOK) {
+ err = DP_ERR_FATAL;
+
+ if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
+ err = DP_ERR_OFFLINE;
+ ctx =
talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
+ struct sdap_id_ctx);
+ if (sdap_check_gssapi_reconnect(ctx)) {
+ if (ctx->gsh) {
+ /* Mark the connection as false so we don't try to
use an
+ * invalid connection by mistake later.
+ * If the global sdap handler is NULL, it's ok not
to do
+ * anything here. It's always checked by
sdap_connected()
+ * before being used.
+ */
+ ctx->gsh->connected = false;
+ }
+ error = sdap_account_info_restart(breq);
+ if (!error) return;
+ }
+ sdap_mark_offline(ctx);
+ }
+ sdap_handler_done(breq, err, ret, errstr);
+ }
+ else sdap_handler_done(breq, DP_ERR_OK, EOK, NULL);
+}
+
static void sdap_account_info_users_done(struct tevent_req *req)
{
struct be_req *breq = tevent_req_callback_data(req, struct be_req);
- struct sdap_id_ctx *ctx;
int dp_err = DP_ERR_OK;
- const char *error = NULL;
- int ret, err;
+ const char *errstr = "Enum Users Failed";
+ int ret;
+ enum account_restart err;
ret = users_get_recv(req);
talloc_zfree(req);
- if (ret) {
- dp_err = DP_ERR_FATAL;
- error = "Enum Users Failed";
-
- if (ret == ETIMEDOUT || ret == EFAULT || ret == EIO) {
- dp_err = DP_ERR_OFFLINE;
- ctx =
talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
- struct sdap_id_ctx);
- if (sdap_check_gssapi_reconnect(ctx)) {
- ctx->gsh->connected = false;
- err = sdap_account_info_restart(breq);
- if (err == EOK) return;
- }
- sdap_mark_offline(ctx);
- }
- }
- sdap_handler_done(breq, dp_err, ret, error);
+ sdap_account_info_common_done(ret, breq, errstr);
}
And then you do not need enum that looks a bit ugly here...
At least this gives the idea of what I mean...
> ------------------------------------------------------------------------
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
