[LinuxCNC/linuxcnc PR#21] added a check if JumpTo folder is owned (writable) by current user, typo fixes

#1 – jepler 于 2015-12-02

I’ve called this pull request to the attention of the primary author of the file iconview.py and asked for his comment.

One small note: Python provides ‘os.access’ to find out whether a location is writable. This may be more efficient than creating a temporary file.

>>> os.access("/tmp", os.W_OK)
True
>>> os.access("/etc", os.W_OK)
False

#2 – gmoccapy 于 2015-12-02

Hallo Balestrino,

I will check this until the weekend and try to merge it. I probally will change to the solution Jeff suggested.
I’ll keep you informed.

Norbert

#3 – Balestrino 于 2015-12-02

I tried with os.access(“tmp”, os.ROK) without success but probably WOK is the right way to test if a dir is accessible from current user.

Anyway Norbert what do you think about saving the JumpTo directory on-the-fly? that would be useful to set the JumpTo folder (ex. USB drives) without restarting LinuxCNC.
If you are agree i can modify that code

#4 – gmoccapy 于 2015-12-03

Hallo Balestrino,

it is nearly done, but I do not understand the part:

        try:
              newiter = self.getiter_last(self.model)
              newpath = self.model.getpath(new_iter)
        except:
              return

Why you put a try in the except: part?
IMHO there should not be a problem, as we filter the error in the path part.

Saving the jump to dir is OK, but it should not be done in the widget itself, it should be done in the surrounding GUI, that is the way I do it in gmoccapy. (onjumptodirchooserfileset())

Norbert

#5 – Balestrino 于 2015-12-03

because if you enter a non owned folder and try to move with “Previous file” you get an error

#6 – gmoccapy 于 2015-12-03

Balestrino,

i changed:

 def onbtnjumptoclicked(self,widget):
       if os.access(self.jumptodir, os.W_OK):
              self.curdir = self.jumpto_dir
              self.fillstore()
              self.btndirup.set_sensitive(True)
      else:
             return False

Nothing else, I do not get an error, as the button jump_to will not change the dir, so IMHO if we get a not acceptable dir, we should deactivate the button.

Norbert

#7 – gmoccapy 于 2015-12-03

I went on with my checks and even if I leave the file unchanged, i am not able to reproduce your error. I can set what ever folder I want using the settings page from gmoccapy, or also setting it as property.

No error at all!

Also write permisions are not needed, as we only read the content.

Could you please give more details?

Norbert

#8 – Balestrino 于 2015-12-04

to reproduce the error go to the Load Program page and navigate to a non owned folder (ex: /root) and “Select previous file”

#9 – gmoccapy 于 2015-12-05

Hallo Balestrino,

OK, now I got it!
That is only for folders where no one has read rights. I found a solution to check that and the corresponing folders “root” and “lost and found” will be skiped, so they are not shown in the widget, is that OK?

Norbert

#10 – Balestrino 于 2015-12-09

Ok Norbert, i’ll close this pull request
thank you

#11 – gmoccapy 于 2015-12-09

OK, then you do agree with my fix?
I pushed it yesterday.

#12 – jepler 于 2015-12-09

Thanks for your work on this issue, @Balestrino and @gmoccapy

#13 – Balestrino 于 2015-12-10

@gmoccapy your fixes are ok. My was only a workaround.
great work Norbert, i love gmoccapy :+1: