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

Fixed segfault in light #6

Merged
merged 17 commits into from
May 3, 2014
Merged

Fixed segfault in light #6

merged 17 commits into from
May 3, 2014

Conversation

Buri
Copy link
Contributor

@Buri Buri commented Apr 26, 2014

  • Minor tweaks and fixes

@pfps
Copy link
Owner

pfps commented Apr 27, 2014

I think that a better way to go is to modify the permissions on the files that
are being fiddled with, so that the program itself does not have to be suid
root. I'll fix up the Makefile to include an install target for the
orientation program that will do this (and get rid of the setup program).

peter

On 04/26/2014 01:37 PM, Jakub Buriánek wrote:

  • Minor tweaks and fixes

    You can merge this Pull Request by running

git pull https://github.com/Buri/yoga-laptop master

Or view, comment on, or merge it at:

#6

    Commit Summary


Reply to this email directly or view it on GitHub
#6.

@pfps
Copy link
Owner

pfps commented Apr 27, 2014

The install script now modifies permissions on the control files to give write
permission to the user's group. That method should let light not need to be
setuid.

I'm not sure just what the current best way to do this is. Perhaps some
policy kit method?

peter

On 04/26/2014 01:37 PM, Jakub Buriánek wrote:

  • Minor tweaks and fixes

    You can merge this Pull Request by running

git pull https://github.com/Buri/yoga-laptop master

Or view, comment on, or merge it at:

#6

    Commit Summary


Reply to this email directly or view it on GitHub
#6.

@Buri
Copy link
Contributor Author

Buri commented Apr 27, 2014

Changing file group will work well for single user computer, however it will break when running in multi user desktop. As short term solution its sufficient, but if we want to make this production ready code, we need to split all three aplications to two group: daemon that reads sensors and send events on bus/produces acpi events/etc. and control application, that will gather the events and make changes. Daemon can run as whatever user and modifiy permissions at will, since nothing else will really read output from sensors directly, however things like backlight are not reserved for theese drivers and therfore I think they should have default permissions unless there is absolute need to change them.

@pfps
Copy link
Owner

pfps commented Apr 27, 2014

Yes, the code should be split as you suggest.

Permissions should be handled via something like policykit, so that the user
who is physically using the screen is the one that can do things like rotating
it or changing brightness. This would mean that thecode doesn't need to run
suid or sgid.

peter

On 04/27/2014 09:33 AM, Jakub Buriánek wrote:

Changing file group will work well for single user computer, however it will
break when running in multi user desktop. As short term solution its
sufficient, but if we want to make this production ready code, we need to
split all three aplications to two group: daemon that reads sensors and send
events on bus/produces acpi events/etc. and control application, that will
gather the events and make changes. Daemon can run as whatever user and
modifiy permissions at will, since nothing else will really read output from
sensors directly, however things like backlight are not reserved for theese
drivers and therfore I think they should have default permissions unless
there is absolute need to change them.


Reply to this email directly or view it on GitHub
#6 (comment).

@Buri
Copy link
Contributor Author

Buri commented Apr 28, 2014

Great, ill look to it in my spare time. :)

@Buri
Copy link
Contributor Author

Buri commented May 2, 2014

Ok, I've fixed some issues in light, cleaned up the code and made separate folder for docs. My next focus will be to do the same for orientation and then ill make the two level split bridged via dbus.

@pfps
Copy link
Owner

pfps commented May 3, 2014

I've pulled these, and (finally) removed the binaries.

peter

On 05/02/2014 08:12 AM, Jakub Buriánek wrote:

Ok, I've fixed some issues in light, cleaned up the code and made separate
folder for docs. My next focus will be to do the same for orientation and then
ill make the two level split bridged via dbus.


Reply to this email directly or view it on GitHub
#6 (comment).

Buri added 2 commits May 3, 2014 09:02
Conflicts:
	sensors/Makefile
	sensors/orientation
@Buri
Copy link
Contributor Author

Buri commented May 3, 2014

Ok, so I have working draft of dbus connection in my local copy. Right now I'm polishing it out and maybe tomorrow I may push it to git.

pfps added a commit that referenced this pull request May 3, 2014
Fixed segfault in light
@pfps pfps merged commit bd0e4a4 into pfps:master May 3, 2014
@pfps
Copy link
Owner

pfps commented May 3, 2014

The directory structure should really be cleaned up at some point. I stupidly put the orientation program in the main directory, which made the drivers a subdirectory of that.

I think that it would be better to put the orientation and light programs in separate subdirectories. What do you think?

@Buri
Copy link
Contributor Author

Buri commented May 7, 2014

Ive made some progress on my branch regarding directory structure. It reflects sensor reader -> worker architecture.
I am now facing following question: use multithreaded blocking read from sensors, or use nonblocking polling?

@pfps
Copy link
Owner

pfps commented May 7, 2014

Hmm. I don't have any guidance here.

peter

On 05/07/2014 06:35 AM, Jakub Buriánek wrote:

Ive made some progress on my branch regarding directory structure. It reflects
sensor reader -> worker architecture.
I am now facing following question: use multithreaded blocking read from
sensors, or use nonblocking polling?


Reply to this email directly or view it on GitHub
#6 (comment).

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.

2 participants