On 27.01.2012 16:04, Nikola Pajkovsky wrote:
-#define PK11_SETATTRS(x,id,v,l) (x)->type = (id); \
- (x)->pValue=(v); (x)->ulValueLen = (l)
please consider to use 'do { ... } while(0)' in macro
#define PK11_SETATTRS(x,id,v,l) \ do { \ (x)->type = (id); \ (x)->pValue = (v); \ (x)->ulValueLen = (l) \ } while(0)
Done.
diff --git a/src/plugins/https-utils.h b/src/plugins/https-utils.h index e69de29..352de40 100644 --- a/src/plugins/https-utils.h +++ b/src/plugins/https-utils.h @@ -0,0 +1,27 @@
license missing and this is header so put around
#ifndef HTTS_UTILS_H #define HTTS_UTILS_H ... #endif
Done.
+#include "libabrt.h"
+extern const char *url;
I don't like the way how it is shared between two different apps.
#define url_var(value) \ static const char *url = value;
Wouldn't work. url is used as a global in both https-utils.c & abrt-{retrace,dedup}-client.c
- if (argc - optind != 2)
show_usage_and_die(usage, options);
clearest way
argv += optind; if (!argv[0]) show_usage_and_die(program_usage_string, program_options);
Done.
- int i;
- /* just a simple check (eg. allows '~' which should not be present in component name) */
- for (i = 0; component[i]; ++i)
if (!noescape(component[i]))
error_msg_and_die(_("Forbidden character in component name: '%c'."), component[i]);
why not?
Naming Guidelines: allowed characters [0-9a-zA-Z_-.+] (ohh, plus :( ) Fixed.
- FILE *fp = fopen(btfile, "r");
- if (!fp)
error_msg_and_die(_("Unable to open file '%s'."), btfile);
- struct strbuf *request_body = strbuf_new();
- strbuf_append_strf(request_body, "component=%s&backtrace=", component);
component memtest89+ wont be working, because '+' is not escaped char *escaped = g_uri_escape_string(component, NULL, 0);
Good point, plus needs to be allowed & escaped. However I don't like the idea of linking with glib because of one function (especially a simple one).
- char buf[1024];
- unsigned char c;
- /* read buffered and escape at the same time */
- /* => just use one strbuf instead of two */
- while (fgets(buf, sizeof(buf), fp))
- {
for (i = 0; buf[i]; ++i)
{
c = (unsigned char)buf[i];
if (noescape(c))
strbuf_append_char(request_body, c);
else
strbuf_append_strf(request_body, "%%%02X", c);
}
- }
also here you can use glib
Same.
- struct dedup_bug_data bug;
- /* iterate line by line */
- char *line = response_body, *newline = response_body;
- int last = 0;
- while (!last)
- {
/* split lines */
while (*newline&& *newline != '\n'&& *newline != '\r')
++newline;
/* end of input => last line */
if (!*newline)
last = 1;
*newline = '\0';
/* we could have stopped at \r, ++newline would be \n */
if (!last)
while (newline[1]&& (newline[1] == '\n' || newline[1] == '\r'))
++newline;
if (sscanf(line, "%d",&bug.bug_id) != 1)
{
line = ++newline;
continue;
}
bug.component = line;
while (*bug.component&& (isspace(*bug.component) || isdigit(*bug.component)))
++bug.component;
bug.status = bug.component;
while (*bug.status&& !isspace(*bug.status))
++bug.status;
*bug.status = '\0';
++bug.status;
if (sscanf(bug.status, "%d",&bug.similarity) != 1)
{
line = ++newline;
continue;
}
while (*bug.status&& (isspace(*bug.status) || isdigit(*bug.status)))
++bug.status;
bug.resolution = NULL;
if (strncmp(bug.status, "CLOSED", strlen("CLOSED")) == 0)
{
bug.resolution = bug.status;
while (*bug.resolution&& !isspace(*bug.resolution))
++bug.resolution;
*bug.resolution = '\0';
++bug.resolution;
while (*bug.resolution&& isspace(*bug.resolution))
++bug.resolution;
}
printf(_("We have detected that your problem is a possible duplicate "
"of bug #%d (from %s).\n"), bug.bug_id, bug.component);
if (bug.resolution)
printf(_("It has already been closed as %s. You can check "
"Bugzilla for a solution.\n"), bug.resolution);
else
printf(_("The bug has not yet been fixed. Feel free to continue "
"reporting and provide as many information as you can.\n"));
line = ++newline;
- }
live is short to write own token parser, use the one from glibc
char *str1, *str2, *token, *subtoken; char *saveptr1, *saveptr2;
for (str1 = t; ; str1 = NULL) { token = strtok_r(str1, "\r\n",&saveptr1); if (token == NULL) break; printf("%s\n", token);
for (str2 = token; ; str2 = NULL) { subtoken = strtok_r(str2, " ",&saveptr2); if (subtoken == NULL) break; printf(" --> %s\n", subtoken); }
}
I agree, it will be more simple using str* functions => needs to be rewritten completely.
/* jump to next line */
while (*cursor&& *cursor != '\n')
++cursor;
++cursor;
simple way
while (t&& *t&& *t++ != '\n') ;
Seems much less readable to me. Anyway, strchr would probably be better: cursor = strchr(cursor, '\n'); if (cursor) ++cursor;