Opened 11 years ago

Closed 8 years ago

#1551 closed defect (obsolete)

filestore.retrieve(): chmod() don't make sense

Reported by: sascha_silbe Owned by: alsroot
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Sugar Version: Git as of bugdate
Severity: Unspecified Keywords:
Cc: Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

While fixing #1550, I stumbled over this code again:

        # Try to hard link from the original file to the targetpath. This can
        # fail if the file is in a different filesystem. Do a symlink instead.
        try:
            os.link(file_path, destination_path)
        except OSError, e:
            if e.errno == errno.EXDEV:
                os.symlink(file_path, destination_path)
            else:
                raise

        # Try to make the original file readable. This can fail if the file is
        # in a FAT filesystem.
        try:
            os.chmod(file_path, 0604)
        except OSError, e:
            if e.errno != errno.EPERM:
                raise

I guess the chmod() is supposed to make the file readable to the caller in case we use a symlink instead of a hardlink.
The permissions don't make much sense:

  • allow read+write for the owner (data store user) - OK
  • no access to the group - ?
  • read access to world - overrides group settings

Was it supposed to be 0640 (rw- r-- ---) instead?

Why do we always do the chmod() instead of only when we actually symlink, BTW? A hardlink doesn't share permissions, only data.

Do we support data stores on FAT filesystems at all? AFAICT our optimizer will fail hard (at least in filestore.hard_link_entry()) on those.

Change History (4)

comment:1 Changed 11 years ago by tomeu

  • Component changed from sugar-datastore to sugar-artwork
  • Owner changed from tomeu to alsroot
  • Status changed from new to assigned

I guess this is from when we had DS in removable devices and we may drop it now.

comment:2 Changed 11 years ago by sascha_silbe

  • Component changed from sugar-artwork to sugar-datastore

I can't imagine what sugar-artwork might have to do with this, so assigning it back to sugar-datastore on the assumption it was a mistake.

comment:3 Changed 8 years ago by dnarvaez

  • Component changed from sugar-datastore to Sugar

comment:4 Changed 8 years ago by dnarvaez

  • Resolution set to obsolete
  • Status changed from assigned to closed

That code doesn't seem to exist anymore.

Note: See TracTickets for help on using tickets.