[ABRT PATCH 1/3] abrt-server: updates/fixes for future rasdaemon needs

Jakub Filak jfilak at redhat.com
Fri Jun 21 13:22:25 UTC 2013


Pushed all. Thanks!

I've added the bugzilla ID (812539) to all of the patches.

The concerns about reporting from anaconda were pointless. We provide
anaconda's reporting configuration and we don't use analyzer there.


On Thu, 2013-06-20 at 16:37 +0200, Denys Vlasenko wrote:
> FILENAME_BACKTRACE and FILENAME_EXECUTABLE make no sense for rasdaemon's
> ECC memory errors reporting - we shouldn't require those.
> 
> Also, we should allow requests with only "type=..."
> but without "analyzer=..." (a step for analyzer -> type transition).
> 
> Bug fix: PUT is the _wrong type of request for creation
> of new entities_! Added POST as an alias.
> 
> Updated abrt-server.txt to show POST and lowercased item names.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk at redhat.com>
> ---
>  doc/abrt-server.txt      | 20 +++++++++----------
>  src/daemon/abrt-server.c | 52 ++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 46 insertions(+), 26 deletions(-)
> 
> diff --git a/doc/abrt-server.txt b/doc/abrt-server.txt
> index 8e9958b..bf9b40d 100644
> --- a/doc/abrt-server.txt
> +++ b/doc/abrt-server.txt
> @@ -38,22 +38,20 @@ connect to UNIX domain socket /var/run/abrt.socket
>  Providing data (writting data to the socket):
>  
>  -------------------------------------------------
> --> "PUT / HTTP/1.1\r\n"
> +-> "POST / HTTP/1.1\r\n"
>  -> "\r\n"
> --> "PID=number\0"
> +-> "type=string\0"
> +   string, maximum length 100 bytes
> +-> "reason=string\0"
> +   string, maximum length 512 bytes
> +-> "pid=number\0"
>     number, 0 - PID_MAX (/proc/sys/kernel/pid_max)
> --> "EXECUTABLE=string\0"
> +-> "executable=string\0"
>     string, maximum length ~MAX_PATH
> --> "BACKTRACE=string\0"
> +-> "backtrace=string\0"
>     string, maximum length 1 MB
> --> "ANALYZER=string\0"
> -   string, maximum length 100 bytes
> --> "BASENAME=string\0"
> -   string maximum length 100 bytes, no slashes
> --> "REASON=string\0"
> -   string, maximum length 512 bytes
>  -> (close writing half of the socket)
> -<- "HTTP/1.1 200 \r\n"
> +<- "HTTP/1.1 201 \r\n"
>  <- "\r\n"
>  -------------------------------------------------
>  
> diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c
> index 98d6db0..14c2f69 100644
> --- a/src/daemon/abrt-server.c
> +++ b/src/daemon/abrt-server.c
> @@ -89,7 +89,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid)
>  
>      gchar *dir_basename = g_hash_table_lookup(problem_info, "basename");
>      if (!dir_basename)
> -        dir_basename = g_hash_table_lookup(problem_info, FILENAME_ANALYZER);
> +        dir_basename = g_hash_table_lookup(problem_info, FILENAME_TYPE);
>  
>      char *path = xasprintf("%s/%s-%s-%u.new",
>                             g_settings_dump_location,
> @@ -232,7 +232,7 @@ static gboolean key_value_ok(gchar *key, gchar *value)
>      /* check value of 'basename', it has to be valid non-hidden directory
>       * name */
>      if (strcmp(key, "basename") == 0
> -     || strcmp(key, FILENAME_ANALYZER) == 0
> +     || strcmp(key, FILENAME_TYPE) == 0
>      )
>      {
>          if (!is_correct_filename(value))
> @@ -268,7 +268,11 @@ static void process_message(GHashTable *problem_info, char *message)
>              else
>              {
>                  g_hash_table_insert(problem_info, key, xstrdup(value));
> -                key = NULL; /* prevent freeing later */
> +                /* Compat, delete when FILENAME_ANALYZER is replaced by FILENAME_TYPE: */
> +                if (strcmp(key, FILENAME_TYPE) == 0)
> +                    g_hash_table_insert(problem_info, xstrdup(FILENAME_ANALYZER), xstrdup(value));
> +                /* Prevent freeing key later: */
> +                key = NULL;
>              }
>          }
>          else
> @@ -289,11 +293,13 @@ static void die_if_data_is_missing(GHashTable *problem_info)
>  {
>      gboolean missing_data = FALSE;
>      gchar **pstring;
> -    static const gchar *const needed[] = {FILENAME_ANALYZER,
> -                                          FILENAME_BACKTRACE,
> -                                          FILENAME_EXECUTABLE,
> -                                          FILENAME_REASON,
> -                                          NULL};
> +    static const gchar *const needed[] = {
> +        FILENAME_TYPE,
> +        FILENAME_REASON,
> +        /* FILENAME_BACKTRACE, - ECC errors have no such elements */
> +        /* FILENAME_EXECUTABLE, */
> +        NULL
> +    };
>  
>      for (pstring = (gchar**) needed; *pstring; pstring++)
>      {
> @@ -408,8 +414,14 @@ static int perform_http_xact(void)
>          return delete_path(messagebuf_data);
>      }
>  
> -    if (strncmp(messagebuf_data, "PUT ", strlen("PUT ")) != 0)
> -    {
> +    /* We erroneously used "PUT /" to create new problems.
> +     * POST is the correct request in this case:
> +     * "PUT /" implies creation or replace of resource named "/"!
> +     * Delete PUT in 2014.
> +     */
> +    if (strncmp(messagebuf_data, "PUT ", strlen("PUT ")) != 0
> +     && strncmp(messagebuf_data, "POST ", strlen("POST ")) != 0
> +    ) {
>          return 400; /* Bad Request */;
>      }
>  
> @@ -464,11 +476,21 @@ static int perform_http_xact(void)
>      die_if_data_is_missing(problem_info);
>  
>      char *executable = g_hash_table_lookup(problem_info, FILENAME_EXECUTABLE);
> -    char *last_file = concat_path_file(g_settings_dump_location, "last-via-server");
> -    int repeating_crash = check_recent_crash_file(last_file, executable);
> -    free(last_file);
> -    if (repeating_crash) /* Only pretend that we saved it */
> -        goto out; /* ret is 0: "success" */
> +    if (executable)
> +    {
> +        char *last_file = concat_path_file(g_settings_dump_location, "last-via-server");
> +        int repeating_crash = check_recent_crash_file(last_file, executable);
> +        free(last_file);
> +        if (repeating_crash) /* Only pretend that we saved it */
> +            goto out; /* ret is 0: "success" */
> +    }
> +
> +#if 0
> +//TODO:
> +    /* At least it should generate local problem identifier UUID */
> +    problem_data_add_basics(problem_info);
> +//...the problem being that problem_info here is not a problem_data_t!
> +#endif
>  
>      ret = create_problem_dir(problem_info, pid);
>  




More information about the Crash-catcher mailing list