Re: [LIBREPORT PATCH] add support for forwarding of report client requests

I don't know this part of abrt/libreport very well, so I'd suggest someone else also take look at the patches as they seem to touch important parts. I haven't found any errors, though I have some personal preferences:

- I don't like callbacks controlled by the *param argument very much. I'd either make another set of callbacks for this purpose, i.e. forwarding as opposed direct interaction w/ user, (possibly sharing code with the old ones) or at least document what the *param does -- the documentation in run_event.h currently claims it is unused. - I'd probably split internationalization of the answers from the rest of the patch.

On Tue, Nov 06, 2012 at 13:56:55 +0100, Jakub Filak wrote:

• required by rhbz#873488

Signed-off-by: Jakub Filak jfilak@redhat.com

src/include/run_event.h | 7 +++++ src/lib/run_event.c | 82 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/src/include/run_event.h b/src/include/run_event.h index c5791f0..8678423 100644 --- a/src/include/run_event.h +++ b/src/include/run_event.h @@ -126,6 +126,13 @@ struct run_event_state { struct run_event_state *new_run_event_state(void); void free_run_event_state(struct run_event_state *state);

+/*

• • Configure callbacks to forward requests
• • @param state A valid run event state pointer
• */

+void make_run_event_state_forwarding(struct run_event_state *state);

/* Asynchronous command execution */

/* Returns 0 if no commands found for this dump_dir_name+event, else >0 */ diff --git a/src/lib/run_event.c b/src/lib/run_event.c index 9c16f75..7aff436 100644 --- a/src/lib/run_event.c +++ b/src/lib/run_event.c @@ -21,6 +21,8 @@ #include "client.h" #include "internal_libreport.h"

+#define FORWARD_REPORT_REQUEST_PTR ((void *)(uintptr_t)2)

struct run_event_state *new_run_event_state() { struct run_event_state *state = xzalloc(sizeof(struct run_event_state)); @@ -46,6 +48,18 @@ void free_run_event_state(struct run_event_state *state) } }

+void make_run_event_state_forwarding(struct run_event_state *state) +{

• /* reset callbacks, just to be sure */
• state->alert_callback = run_event_stdio_alert;
• state->ask_callback = run_event_stdio_ask;
• state->ask_yes_no_callback = run_event_stdio_ask_yes_no;
• state->ask_yes_no_yesforever_callback= run_event_stdio_ask_yes_no_yesforever;
• state->ask_password_callback = run_event_stdio_ask_password;
• /* notify default callbacks */
• state->interaction_param = FORWARD_REPORT_REQUEST_PTR;

+}

/* Asynchronous command execution */

@@ -508,13 +522,13 @@ int consume_event_command_output(struct run_event_state *state, const char *dump key_end[0] = ' '; /* restore original message, not sure if it is necessary */ }

•            response = xstrdup(ans ? "yes" : "no");
  

•            response = xstrdup(ans ? _("y") : _("N"));
         }
         /* wait for y/N response on the same line */
         else if (prefixcmp(msg, REPORT_PREFIX_ASK_YES_NO) == 0)
         {
             const bool ans = state->ask_yes_no_callback(msg + sizeof(REPORT_PREFIX_ASK_YES_NO) - 1, state->interaction_param);
  

•            response = xstrdup(ans ? "yes" : "no");
  

•            response = xstrdup(ans ? _("y") : _("N"));
         }
         /* wait for the string on the same line */
         else if (prefixcmp(msg, REPORT_PREFIX_ASK) == 0)
  

@@ -675,13 +689,21 @@ char *list_possible_events(struct dump_dir *dd, const char *dump_dir_name, const

void run_event_stdio_alert(const char *msg, void *param) {

• printf("%s\n", msg);

• if (param == FORWARD_REPORT_REQUEST_PTR)

•    printf(REPORT_PREFIX_ALERT "%s\n", msg);
  

• else

•    printf("%s\n", msg);
  

• fflush(stdout);

}

-char *run_event_stdio_ask(const char *msg, void *param) +static char *internal_ask(const char *msg, const char *prefix) {

• printf("%s ", msg);

• if (prefix == NULL)

•    printf("%s ", msg);
  

• else

•    printf("%s%s\n", prefix, msg);
  

• fflush(stdout); char buf[256]; if (!fgets(buf, sizeof(buf), stdin))

@@ -692,26 +714,48 @@ char *run_event_stdio_ask(const char *msg, void *param) return xstrdup(buf); }

+char *run_event_stdio_ask(const char *msg, void *param) +{

• return internal_ask(msg, param == FORWARD_REPORT_REQUEST_PTR ? REPORT_PREFIX_ASK : NULL);

+}

int run_event_stdio_ask_yes_no(const char *msg, void *param) {

• printf("%s [%s/%s] ", msg, _("y"), _("N"));

• const char *yes = _("y");
• const char *no = _("N");
• if (param == FORWARD_REPORT_REQUEST_PTR)

•    printf(REPORT_PREFIX_ASK_YES_NO "%s\n", msg);
  

• else

•    printf("%s [%s/%s] ", msg, yes, no);
  

• fflush(stdout); char buf[16]; if (!fgets(buf, sizeof(buf), stdin)) buf[0] = '\0';

• return buf[0] == 'y' && (buf[1] == '\n' || buf[1] == '\0');

• return strncmp(yes, buf, strlen(yes)) == 0;

}

int run_event_stdio_ask_yes_no_yesforever(const char *msg, const char *key, void *param) {

• const char *ask_result = get_user_setting(key);

• const char *yes = _("y");
• const char *no = _("N");
• const char *forever = _("f");

• if (ask_result && string_to_bool(ask_result) == false)

•    /* Do you want to be asked? -> No, I don't */
  

•    return 1;
  

• if (param == FORWARD_REPORT_REQUEST_PTR)

•    printf(REPORT_PREFIX_ASK_YES_NO_YESFOREVER "%s %s\n", key, msg);
  

• else
• {

•    const char *ask_result = get_user_setting(key);
  

•    if (ask_result && string_to_bool(ask_result) == false)
  

•        /* Do you want to be asked? -> No, I don't */
  

•        return 1;
  

•    printf("%s [%s/%s/%s] ", msg, yes, no, forever);
  

• }

• printf("%s [%s/%s/%s] ", msg, _("y"), _("N"), _("f")); fflush(stdout); char buf[16]; if (!fgets(buf, sizeof(buf), stdin))

@@ -721,17 +765,21 @@ int run_event_stdio_ask_yes_no_yesforever(const char *msg, const char *key, void if (endln) endln[0] = '\0';

• /* Do you want to be asked for next time? */
• /* 'f' means 'yes forever' and it means 'no, dont' ask me again' */
• set_user_setting(key, strcmp(_("f"), buf) == 0 ? "no" : "yes");

• if (param != FORWARD_REPORT_REQUEST_PTR)
• {

•    /* Do you want to be asked for next time? */
  

•    /* 'f' means 'yes forever' and it means 'no, dont' ask me again' */
  

•    set_user_setting(key, strncmp(forever, buf, strlen(forever)) == 0 ? "no" : "yes");
  

• }

• return strcmp(_("f"), buf) == 0 || strcmp(_("y"), buf) == 0;

• return strncmp(forever, buf, strlen(forever)) == 0 || strncmp(yes, buf, strlen(yes)) == 0;

}

char *run_event_stdio_ask_password(const char *msg, void *param) { const bool changed = set_echo(false);

• char *const password = run_event_stdio_ask(msg, param);

• char *const password = internal_ask(msg, param == FORWARD_REPORT_REQUEST_PTR

•                                            ? REPORT_PREFIX_ASK_PASSWORD : NULL);
  
  

  if (changed) set_echo(true);

-- 1.7.11.7