#2415 assigned defect

Duplicate code in Untar_FromMemory(), Untar_FromFile() and rtems_tarfs_load()

Reported by: Sebastian Huber Owned by: Needs Funding
Priority: normal Milestone: Indefinite
Component: unspecified Version: 4.10
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

These functions should use a general purpose TAR iterator function with specialized visitors.

Change History (6)

comment:1 Changed on May 15, 2016 at 11:51:27 PM by Chris Johns

Reviewing the code I suspect it might be difficult to merge the functionality as things exist.

The loading of a tar file into the IMFS seems to be mixing POSIX functionality and internal IMFS functionality. The IMFS tar file load code directly references the tar image for the file contents. The referencing is nice where RAM usage is important but there are assumptions being made such as the storage for the tar file has to remain valid for the life time of the mount and there is no other activity in the IMFS when this call is made. I am also not sure how locking in the file system is managed when the references to the file contents are being made. Is this code thread safe in regards to the operation of the IMFS?

The libmisc untar code does not handle the full tar semantics when updating an existing directory tree. Ticket #2207 covers this issue. I am not sure how these semantics would relate to the IMFS implementation if that is at all possible.

I think special access to the IMFS to allow neat features such as adding a file node based on a piece of memory is a good idea but I think having things like tar code in the IMFS file system tree and a direct user API call is wrong. I wonder is the IMFS should be given a /dev node and ioctl calls made to access any special features. This would provide us with a robust user interface we could control. Once this is done we would be in a better position to provide a common implementation for tar loading as well as supporting the full tar semantics.

comment:2 Changed on May 17, 2016 at 5:47:45 AM by Sebastian Huber

We should at least refactor the general TAR file iteration into an iterator function and use different visitor functions.

Another option for this direct mapping of some read-only content into an IMFS linear file would be mmap().

comment:3 in reply to:  2 Changed on May 17, 2016 at 5:55:33 AM by Chris Johns

Replying to sebastian.huber:

We should at least refactor the general TAR file iteration into an iterator function and use different visitor functions.

I have changed untar.c (need to post for review) to have a ProcessHeader? call and the in-memory and file versions now share that code to handle the semantics. The only differences are the writes to the files. Further work could be done to make the code share more if there is a need. The important part is shared.

Another option for this direct mapping of some read-only content into an IMFS linear file would be mmap().

[ implementation here https://git.rtems.org/chrisj/rtl.git/tree/mmap.c ]

I think this is the other way around. The current tar functionality takes some memory and makes files in the IMFS while mmap takes a file and makes it appear as memory.

comment:4 Changed on Jun 4, 2016 at 12:11:31 AM by Chris Johns <chrisj@…>

In d84e346b26017f021c1a7d5c8ad078c7264240ab/rtems:

libmisc/untar: Support directory create and overwrites. Share the common code.

Support creating directories for files with a path depth greater than 1. Some
tar files can have files with a path depth greater than 1 and no directory
entry in the tar file to create a directory.

Support overwriting existing files and directories failing in a similar
way to tar on common hosts. If a file is replaced with a file delete the
file and create a new file. If a directory replaces a file remove the file
and create the directory. If a file replaces a directory remove the directory,
and if the directory is not empty and cannot be removed report an error. If a
directory alreday exists do nothing leaving the contents untouched.

The untar code now shares the common header parsing and initial processing
with the actual writes still separate. No changes to the IMFS have been made.

Updates #2415.
Closes #2207.

comment:5 Changed on Jan 26, 2017 at 7:16:00 AM by Sebastian Huber

Milestone: 4.11.14.11.2

comment:6 Changed on Feb 15, 2017 at 1:37:51 PM by Sebastian Huber

Milestone: 4.11.2Indefinite
Owner: set to Needs Funding
Status: newassigned
Note: See TracTickets for help on using tickets.