Bug hunting #125
Bug hunting #125
Conversation
|
It's easier to navigate the commits in G-P if you prefix the commit message with the name of the plugin. |
|
Can you please rebase the patchset on current HEAD? Some of the patches have been already been addressed. |
|
Git says that current branch is up to date, isn't? Maybe I fixed some previous fixes (e.g. mismatching allocator/deallocator)? |
|
My fault. I thought some of the fixes were already in git/master but I was wrong. Sry. |
|
I have applied the patches for my plugins. Not sure at the moment what I will do with the others. |
| @@ -114,7 +114,11 @@ static void on_target_browse_clicked(GtkButton *button, gpointer user_data) | |||
| if (strcmp(".", prevdir)) | |||
| strcpy(path, prevdir); | |||
| else | |||
| strcpy(path, g_path_get_dirname(DOC_FILENAME(document_get_current()))); | |||
| { | |||
| gchar dirname = g_path_get_dirname(DOC_FILENAME(document_get_current())); | |||
|
Okay maybe this is a hard to review, but I keep this because I cannot login to SF to leave a bug report and there is no Issues section. ML is a black hole and not suitable as a tracker:) |
| g_error_free(error); error = NULL; | ||
| return success; | ||
| } | ||
| g_return_val_if_fail(contents, success); |
There was a problem hiding this comment.
This is wrong IMO. The g_return_* functions are for programmer errors. Here, this is a runtime error.
Also, I do not see any benefit of removing the error message.
There was a problem hiding this comment.
Regarding to documentation on function g_key_file_to_data:
Note that this function never reports an error, so it is safe to pass NULL as error.
So it's ok to remove dead code.
There was a problem hiding this comment.
Indeed, but it's also useless to add the check -- this function never returns NULL either. And as @sardemff7 said, it's kinda weird to use g_return_if_fail() here, which is supposed to be used to check for programming errors like incorrect parameters -- ok, here it checks that g_key_file_to_data() isn't buggy, but huh. IMHO, either leave it as is with the useless error checking, or remote the thing entirely.
Bugs and leaks found by cppcheck.