Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

Check access rights of config files before opening them #85

Merged
merged 6 commits into from
Jul 30, 2018

Conversation

DonatJR
Copy link
Contributor

@DonatJR DonatJR commented Jul 26, 2018

fixes #75

  • Configurator and Service now check the access rights of the config file before opening / processing them further
  • The list of available configs in the Chooser is now based on registry entries written by the Configurator
  • Configs are now all stored in the same folder to make it less likely a registry entry gets invalidated

@bencikpeter Could you take a look at the CheckConfigAccessRights function in the CageService? You have the most experience with the Windows API in this regard
https://github.com/SharkCagey/HTWG_shark_cage/pull/85/files#diff-c70ab997f5ee033d6f6d8bd7ca7264b8R243

Copy link
Contributor

@SailReal SailReal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thx for the awesome work...a few points to discuss:

  • Maybe we can introduce an Refresh button in the Cage Chooser which will do the same as the 'F5' keystroke?
  • Maybe we can set the current size as the max size of the Configurator otherwise is it much to do to make it more responsive?
  • Maybe we can display a simple text in the Configurator, that the saving was successfully?

@SailReal
Copy link
Contributor

Oh and we have a conflict file "SharkCage/CageChooser/CageChooserForm.cs"

@DonatJR
Copy link
Contributor Author

DonatJR commented Jul 26, 2018

@SailReal

  1. that was my original plan, but then I wanted a nice refresh icon instead of just plain text which means doing it myself or finding a good looking icon with a suitable license - too much work for now 😅 the list also refreshes on restart so I don't think the button will be absolutely necessary
  2. I will constrain it to the current size. Getting all controls to size with the form is possible I think, but I don't know how to do it (yet) -> too much work ;)
  3. Great idea!

DonatJR added 3 commits July 27, 2018 09:28
change order in CageManager for faster message processing on startup
enter button in CageChooser now does the same as pressing the start button
Copy link
Contributor

@SailReal SailReal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice work!

@DonatJR DonatJR merged commit bc56a0d into develop Jul 30, 2018
@DonatJR DonatJR deleted the 75-config-file-acl branch July 30, 2018 14:16
@bencikpeter
Copy link
Contributor

@DonatJR Sorry for a late reply, I somehow overlooked that mention

I had a look at CheckConfigAccessRights And I have fer remarks

  • Is there any particular reason for using ::LocalFree and ::LocalAlloc? Documentation states that

The local functions have greater overhead and provide fewer features than other memory management functions. New applications should use the heap functions unless documentation states that a local function should be used.

        so I´d assume it´s a better idea to use those?

  • variable name of access_rights_okay is a bit misleading if the value is false. Maybe something along the lines of access_rights_status would be more appropriate?

  • from a functional point of view, I don´t see anything wrong with it, it should be fine.

DonatJR added a commit that referenced this pull request Jul 31, 2018
DonatJR added a commit that referenced this pull request Jul 31, 2018
minor improvements from general discussion and #85
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants