gEDA-bug: [Bug 698565] Re: Save button doesn't respect permissions

[Ivan Stankovic <698565@xxxxxxxxxxxxxxxxxx>]

I took a little time to investigate this. Running strace -e gschem when saving a file gives this output
(generated by the f_save function):

lstat("/tmp/foo.sch", {st_mode=S_IFREG|0644, st_size=45, ...}) = 0
   => produced by follow_symlinks

access("/tmp/foo.sch", F_OK)            = 0
stat("/tmp/foo.sch", {st_mode=S_IFREG|0644, st_size=45, ...}) = 0
  => produced by g_file_test

access("/tmp/foo.sch~", F_OK)           = 0
stat("/tmp/foo.sch~", {st_mode=S_IFREG|0444, st_size=211, ...}) = 0
  => same here, but for the backup file

chmod("/tmp/foo.sch~", 0600)            = 0
  => make the previous backup file readable and writable by the user (why?)

rename("/tmp/foo.sch", "/tmp/foo.sch~") = 0
  => this is the interesting part: the existing schematic is renamed to the backup file;
        note that the backup file will have the same permissions as the original file...

chmod("/tmp/foo.sch~", 0444)            = 0
  => ... until this chmod executes; so far so good.

Then the weird part:

stat("/tmp/foo.sch", 0x7fffbfe1f2e0)    = -1 ENOENT (No such file or directory)
  => we're giving stat the real_filename, which has just been renamed to backup file,
        so it's no surprise that stat returns ENOENT; The comment just above that stat
        says

         /* If there is not an existing file with that name, compute the
          * permissions and uid/gid that we will use for the newly-created file.
          */

        Now I'd say that this is a bug and that stat() should have been called before renaming,
        Or is there a good reason for this behaviour?

But that isn't what really causes the bug we're talking about. The real bug is
that there is no checking for permissions on the existing file. f_save calls o_save which
uses g_file_set_contents which will happily rename() the temporary file to the existing file,
 ignoring permissions and the resulting file will have the same permissions it had before
(by default read+write).

So the fix would be to have a function that checks the permissions and call that function
before calling f_save. Overall, I think f_save is a complex block of code and tries to do a bit
too much (do we really have to mess with chmod, umask, user ids, group ids etc.?).
And I can't really understand why the backup functionality is there in f_save, or libgeda
for that matter...

-- 
You received this bug notification because you are a member of gEDA Bug
Team, which is subscribed to gEDA.
https://bugs.launchpad.net/bugs/698565

Title:
  Save button doesn't respect permissions

Status in GPL Electronic Design Automation tools:
  Triaged

Bug description:
  File->Save _sometimes_ fails to respect file-permissions.

  For example, when modifying a file with permissions:
  -r--r--r-- 1 pcjc2 pcjc2 510 2008-12-27 10:23 /home/pcjc2/geda/share/gEDA/sym/analog/resistor-2.sym

  The log window often gets the following text:
  o_save: Could not open [/home/pcjc2/geda/share/gEDA/sym/analog/resistor-2.sym]
  Could NOT save page [/home/pcjc2/geda/share/gEDA/sym/analog/resistor-2.sym]
  (Arguably we just need one descriptive message with a reason).

  However; on some occasions, the file gets overritten, creating a new file with permissions:
  -rw-r--r-- 1 pcjc2 pcjc2 490 2008-12-27 10:29 /home/pcjc2/geda/share/gEDA/sym/analog/resistor-2.sym

  Obviously this is not good!

To manage notifications about this bug go to:
https://bugs.launchpad.net/geda/+bug/698565/+subscriptions


_______________________________________________
geda-bug mailing list
geda-bug@xxxxxxxxxxxxxx
http://www.seul.org/cgi-bin/mailman/listinfo/geda-bug