Jiri Moskovcak jmoskovc@redhat.com writes:
Please see the comments inline.
Jirka
On 11/29/2011 03:26 PM, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovskynpajkovs@redhat.com
- if you call init() shouldn't there be deinit() somewhere?
for short time running app it doesn't make sense to free all variables, because the os will do that.
- int best_bt_rating = 0;
- for (int i = 0; i< comments_memb_size; ++i)
- {
xmlrpc_value* item = NULL;
xmlrpc_array_read_item(&env, comments_memb, i,&item);
if (env.fault_occurred)
abrt_xmlrpc_die(&env);
char *comment_body = rhbz_bug_read_item("body", item, RHBZ_READ_STR);
/* attachments are sometimes without comments -- skip them */
if (!comment_body)
continue;
char *start_rating_line = strstr(comment_body, "rating: ");
if (!start_rating_line)
{
error_msg("no rating");
- use VERBX log() here, it's not an error when there is no "rating: "
in some comment
yep
continue;
}
start_rating_line += strlen("rating: ");
char *end_rating_line = strchr(start_rating_line, '\n');
if (!end_rating_line)
error_msg_and_die("broken comment body");
- I wouldn't die here, what if the next comment body will be ok? and
you still might want to continue with adding the current reporter to CC
yep
char *rating_line = xstrndup(start_rating_line, end_rating_line - start_rating_line);
- at this point I would use char *rating_str instead of rating_line
- and you leak rating_line
why not. same as init() deinit() xmlrpc
int old_errno = errno;
- wtf? why you need to store errno?
errno = 0;
char *e;
long rating = strtoul(rating_line,&e, 10);
if (errno || rating_line == e || *e != '\0' || rating> UINT_MAX)
{
/* error / no digits / illegal trailing chars */
errno = old_errno;
-- and restore it ^?
continue;
}
errno = old_errno; /* Ok. So restore errno. */
- why?^
Since strtol() can legitimately return 0, LONG_MAX, or LONG_MIN (LLONG_MAX or LLONG_MIN for strtoll()) on both success and failure, the calling program should set errno to 0 before the call, and then determine if an error occurred by checking whether errno has a non-zero value after the call.