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

QNetMDTracksModel: Avoid crash due to permission error #58

Merged
merged 1 commit into from Mar 29, 2020

Conversation

@thp
Copy link
Collaborator

thp commented Mar 23, 2020

This fixes a crash when the current user doesn't have permission to open the NetMD device.

Steps to reproduce:

  1. Remove the /etc/udev/rules.d/netmd.rules file or remove the user from the plugdev group.
  2. Run QHiMDTransfer
@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Mar 24, 2020

@gavinbenda can you merge these fixes in your Platinum-MD ?

@gavinbenda

This comment has been minimized.

Copy link

gavinbenda commented Mar 24, 2020

@gavinbenda can you merge these fixes in your Platinum-MD ?

I really only use netmdcli in Platinum-MD - I don't think this code is included anywhere, as it's part of qhimdtransfer in linux-minidisc.

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Mar 24, 2020

@gavinbenda Understood. I see finally some new developments in the minidisc code so I was wondering if you are keeping track of it.

@glaubitz

This comment has been minimized.

Copy link
Collaborator

glaubitz commented Mar 27, 2020

@gavinbenda Plantinum-MD looks great. Glad that our code is useful for other projects, too.

If you have any improvements for the linux-minidisc code, feel free to open PRs.

I have asked @vuori to merge upstream his improvements so that we can reduce merge all the improvements people made back upstream. I'm very happy to see all these improvments!

Copy link
Collaborator

glaubitz left a comment

@thp Is there a reason why we don't immediately want to return from QNetMDTracksModel when device is NULL?

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Mar 27, 2020

@glaubitz Can you help Gavin port downloackhack.py into Platinum-MD please? He does not have an MZ-RH1 to test. I do have one but lack all the skills you guys have. I'm still using the @thp testing version that has upload functionality.

@glaubitz

This comment has been minimized.

Copy link
Collaborator

glaubitz commented Mar 27, 2020

@Invictaz The Python code is not really my terrain. It was supposed to be a proof-of-concept only back when it was written. The functionality was later ported to libnetmd, albeit some features in libnetmd need more fixing, but @vuori has fixed many issues.

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Mar 28, 2020

@glaubitz Thanks for the answer. To my knowledge that downloadhack.py was not touched in ages.
Users e-mail me because they do not listen to music anymore on the minidisc, but use the RH1 for archival purposes. With the Minidisc becoming older a valid archival method is necessary.
SonicStage 6.0 was only Japanese and I failed to install it, even after modifying some ini files.

@thp

This comment has been minimized.

Copy link
Collaborator Author

thp commented Mar 28, 2020

@Invictaz Please file a separate issue for the downloadhack.py-to-libnetmd porting.

@thp thp force-pushed the netmd-permissions-crash branch from 6d62397 to 566d887 Mar 28, 2020
@thp

This comment has been minimized.

Copy link
Collaborator Author

thp commented Mar 28, 2020

@glaubitz Updated PR.

  1. device and ndev are actually non-NULL (wrong assumption on my part)
  2. ret = ndev->open(); returns a non-empty string on permission error
  3. if(!ret.isEmpty()) close(); causes ndev to become NULL

So the fix for this particular issue was to return ret; when we close() due to an error message returned from ndev->open().

@glaubitz

This comment has been minimized.

Copy link
Collaborator

glaubitz commented Mar 28, 2020

Great. Looks good to me. Shall I merge or do you want to merge it yourself?

@thp thp merged commit 441025f into master Mar 29, 2020
@thp thp deleted the netmd-permissions-crash branch Mar 29, 2020
@vuori

This comment has been minimized.

Copy link

vuori commented Mar 29, 2020

re: downloadhack, I haven't touched that and I don't think it's really useful except as reference. It should probably be removed or moved to some "old stuff" directory at some point so people don't try to use it.

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Mar 29, 2020

@vuori it's the best we have to get songs of the device. If I could be of any assistance (without lending or destroying my MZ-RH1) let me know.

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.