Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove use of g_error() #222

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

LaserEyess
Copy link
Member

@LaserEyess LaserEyess commented Apr 15, 2023

Fixes #221

@LaserEyess LaserEyess force-pushed the remove_g_error branch 3 times, most recently from 2b53125 to 5f4229a Compare April 15, 2023 16:22
@LaserEyess LaserEyess marked this pull request as ready for review April 15, 2023 16:22
@LaserEyess
Copy link
Member Author

@van-de-bugger please try this to make sure it fixes it for you. It should pop up an error dialog if there's a permissions issue. The error message is not the best, but it's what glib returns.

src/util.c Outdated Show resolved Hide resolved
src/trg-prefs.c Outdated Show resolved Hide resolved
g_error is only supposed to be for unexpected errors, and therefore
should not be used for expected errors. In torrent-add-dialog, the
specific issue is that not having read permissions on a file is indeed
an *expected* error. In trg_base64encode, not being able to base64encode
a file for some reason is also expected. These errors should be passed
back to the user,

Some refactoring was needed to create a general error-dialog message.
There are also still unrelated g_error() usages that need to be cleaned
up.
In trg_prefs_constructor, g_get_user_config_dir() is used to create
priv->file. This value is always what XDG_CONFIG_HOME is, or at least
what the default is, e.g. ~/.config. There should be no circumstances,
outside of explicit user intervention, for this to not be user
writeable.

That being said, if a permissions issue existed, the code was set to
core dump via g_error(). That really shouldn't be the response to this
as there are cases where this could happen, though it is almost
certainly user error outside TRG's control. Therefore, bump this down to
a warning. If someone hits this, they'll be able to see what the warning
is and understand it.
@van-de-bugger
Copy link

@van-de-bugger please try this to make sure it fixes it for you. It should pop up an error dialog if there's a permissions issue. The error message is not the best, but it's what glib returns.

I am not familiar with git and git hub. I have managed to checkout the mater branch, but I have no idea how to checkout your changes.

@LaserEyess
Copy link
Member Author

LaserEyess commented Apr 16, 2023

@LaserEyess LaserEyess merged commit 03e851f into transmission-remote-gtk:master Apr 19, 2023
@van-de-bugger
Copy link

van-de-bugger commented May 31, 2023

Hi, sorry for delay.

Ok, with the patch TRG does not crash but shows a dialog box with decent error message.

There are few problems, though:

  1. When error box is closed, TRG continues as if the torrent file was successfully open and shows "Add torrent" dialog with empty file list. Since torrent file was not read, there is no any sense to show this dialog.
  2. If I am trying to open more than one unreadable torrent files, the error box is not shown at all (something flickers on the screen, but my eye is not so fast to see what it is there), but "Add torrent" dialog is still shown.

Also, error message is a bit redundant:

Failed to open "...": open() failed: Permission denied

It says the same info twice: "failed to open" and "open() failed" is just a tautology. I understand failed function name may be meaningful for a developer, but it is too cryptic for an end user. That's not a big problem, though.

@LaserEyess
Copy link
Member Author

Can you open an issue for each of those separately?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usage of g_error in source code causes core dumps
3 participants