On Wednesday 15 of August 2012 21:02:44 Jiri Moskovcak wrote:
> ---
>  src/include/problem_data.h |  1 +
>  src/lib/abrt_sock.c        | 89
> +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 82
> insertions(+), 8 deletions(-)
> 
> diff --git a/src/include/problem_data.h b/src/include/problem_data.h
> index d741b5d..0392e0f 100644
> --- a/src/include/problem_data.h
> +++ b/src/include/problem_data.h
> @@ -89,6 +89,7 @@ char *problem_data_get_content_or_NULL(problem_data_t
> *problem_data, const char /* Aborts if key is not found: */
>  char *problem_data_get_content_or_die(problem_data_t *problem_data, const
> char *key);
> 
> +int problem_data_send_to_abrt(problem_data_t* problem_data);

Style violation  ... problem_data_t*

> 
>  /* Conversions between in-memory and on-disk formats */
> 
> diff --git a/src/lib/abrt_sock.c b/src/lib/abrt_sock.c
> index b595208..d52b975 100644
> --- a/src/lib/abrt_sock.c
> +++ b/src/lib/abrt_sock.c
> @@ -21,18 +21,37 @@
> 
>  #define SOCKET_FILE  VAR_RUN"/abrt/abrt.socket"
> 
> -static int connect_to_abrtd_and_call_DeleteDebugDump(const char
> *dump_dir_name) +/* conencts to abrtd
> + * returns: socketfd
> + * -1 on error
> + */
> +static int connect_to_abrtd_socket()
>  {
> -    int result = 1; /* error so far */
> -
>      int socketfd = xsocket(AF_UNIX, SOCK_STREAM, 0);
> +    if (socketfd == -1)
> +        return -1;
>      /*close_on_exec_on(socketfd); - not needed, we are closing it soon */
>      struct sockaddr_un local;
>      memset(&local, 0, sizeof(local));
>      local.sun_family = AF_UNIX;
>      strcpy(local.sun_path, SOCKET_FILE);
>      int r = connect(socketfd, (struct sockaddr*)&local, sizeof(local));
> -    if (r == 0)
> +    if (r != 0)
> +        return -1;
> +
> +    return socketfd;
> +}
> +
> +static int connect_to_abrtd_and_call_DeleteDebugDump(const char
> *dump_dir_name) +{
> +    int result = 1; /* error so far */
> +    int socketfd = connect_to_abrtd_socket();
> +    if (socketfd == -1)
> +    {
> +        VERB1 perror_msg("Can't connect to '%s'", SOCKET_FILE);
> +        return result;
> +    }
> +    else
>      {
>          full_write(socketfd, "DELETE ", strlen("DELETE "));
>          full_write(socketfd, dump_dir_name, strlen(dump_dir_name));
> @@ -40,7 +59,7 @@ static int connect_to_abrtd_and_call_DeleteDebugDump(const
> char *dump_dir_name) shutdown(socketfd, SHUT_WR);
> 
>          char response[64];
> -        r = full_read(socketfd, response, sizeof(response) - 1);
> +        int r = full_read(socketfd, response, sizeof(response) - 1);
>          if (r >= 0)
>          {
>              VERB1 log("Response via socket:'%.*s'", r, response);
> @@ -51,13 +70,67 @@ static int
> connect_to_abrtd_and_call_DeleteDebugDump(const char *dump_dir_name) result
> = strncmp(response, "HTTP/1.1 200 ", strlen("HTTP/1.1 200 ")); }
>      }
> -    else
> +
> +    close(socketfd);
> +
> +    return result;
> +}
> +
> +int problem_data_send_to_abrt(problem_data_t* problem_data)

Style violation ... problem_data_t*


> +{
> +    int result = 1; /* error so far */
> +    int socketfd = connect_to_abrtd_socket();
> +    if (socketfd == -1)
>      {
>          VERB1 perror_msg("Can't connect to '%s'", SOCKET_FILE);
> +        return result;
>      }
> -    close(socketfd);
> +    else
> +    {
> +        GHashTableIter iter;
> +        char *name;
> +        struct problem_item *value;
> +        g_hash_table_iter_init(&iter, problem_data);
> 
> -    return result;
> +        full_write(socketfd, "PUT / HTTP/1.1\r\n\r\n", strlen("PUT /
> HTTP/1.1\r\n\r\n")); +        while (g_hash_table_iter_next(&iter,

Is it necessary to use strlen() with literals? I'm not user if compiler is so 
smart to replace it with sizeof() :)

> (void**)&name, (void**)&value)) +        {
> +            if (value->flags & CD_FLAG_BIN)
> +            {
> +                /* sending files over the socket is not implemented yet */
> +                log("Skipping binary file %s", name);
> +                continue;
> +            }
> +
> +            /* only files should contain '/' and those are handled earlier
> */ +            if (name[0] == '.' || strchr(name, '/'))
> +            {
> +                error_msg("Problem data field name contains disallowed
> chars: '%s'", name); +                continue;
> +            }
> +
> +            char* msg = xasprintf("%s=%s", name, value->content);

Style violatin ... char*

Note: It's a pity to return a malloced string and then compute a length when 
asprintf() returns length of string on its own.

> +            full_write(socketfd, msg, strlen(msg)+1 /* yes, +1 coz we want
> to send the trailing 0 */); +            free(msg);
> +        }
> +        shutdown(socketfd, SHUT_WR);
> +
> +        char response[64];
> +        int r = full_read(socketfd, response, sizeof(response) - 1);
> +        if (r >= 0)
> +        {
> +            VERB1 log("Response via socket:'%.*s'", r, response);
> +            /*  0123456789...  */
> +            /* "HTTP/1.1 200 " */
> +            response[5] = '1';
> +            response[7] = '1';
> +            result = strncmp(response, "HTTP/1.1 201 ", strlen("HTTP/1.1
> 201 ")); +        }

I would rather use prefixcmp() instead of strncmp().

> +
> +        close(socketfd);
> +
> +        return result;
> +    }
>  }
> 
>  int delete_dump_dir_possibly_using_abrtd(const char *dump_dir_name)

Reply via email to