On Mon, Nov 23, 2015 at 08:49:44AM +0100, Jean-Pierre André wrote:
> See proposal on
> http://jp-andre.pagesperso-orange.fr/systcomp.patches.gz
Hi,
Great work! It looks cleaner than I had expected. I have a few questions and
comments:
Should plugin_operations_t really be declared in reparse.h? reparse.h is
supposed to be a header for the library, but the plugins are specific to the
FUSE driver.
I think the plugins will need to always be loaded from a fixed location set
during the build. The 'launch_path' logic effectively allows users to load
arbitrary plugins into ntfs-3g, which is a problem if someone has ntfs-3g
installed setuid root.
Naming: should the word "reparse" be included? For example:
"ntfs_reparse_plugin_operations_t", "ntfs_plugin_reparse_80000017.so". In the
code, just calling them "plugins" seems too vague. Of course, the way the
plugins are presented to users can still be feature-oriented, e.g. "this NTFS-3g
plugin allows you to read system-compressed files".
With a reparse plugin for tag 0x80000017 I will get sent all the WOF reparse
points, including WIM-backed files ("WIMBoot" files) and anything Microsoft may
add to WOF in the future, not just system-compressed files. On unsupported
files, should I just fail operations with EOPNOTSUPP? Or should I pretend the
file is a symlink to "unsupported reparse point"?
There is an edge case in ntfs_get_reparse_tag(). If the reparse tag is 0, the
function returns 0 but doesn't set errno. This could be fixed by having
valid_reparse_data() consider reparse tags of 0 to be invalid. layout.h already
defines the constant IO_REPARSE_TAG_RESERVED_ZERO, and Windows considers this
value to be invalid as well.
In the FUSE driver, there could be a reusable function which combines
ntfs_get_reparse_tag() and get_reparse_plugin().
reparse_getattr() and reparse_readlink() sound overly generic, since they don't
handle all types of reparse points. Maybe they should be renamed to
symlink_getattr() and symlink_readlink(). Or perhaps
link_reparse_point_getattr() and link_reparse_point_readlink()?
The reparse plugin operations need to be clearly documented so that plugin
authors know what to do.
It probably makes the most sense to pass the reparse data to the plugin
operations. Otherwise, almost every operation would end up immediately reading
the reparse data anyway. There would, however, need to be a new version of
ntfs_make_symlink() that takes in the reparse data directly.
With the proposed hook placement in ntfs_fuse_read(), there is no way for the
plugin to support reading named data streams. For system compressed files, only
the default behavior is needed. So to fix this, it would be sufficient to only
call the plugin if the unnamed data stream is being read. That may be fine, and
ntfs_fuse_getattr() already has similar behavior. But I don't know whether that
behavior is necessarily what all plugins will want.
There is an open() plugin operation, but isn't called from anywhere.
Do plugins need to export a "cleanup" or "deinit" function to go along with
"init"? Or should it be assumed that either plugins won't allocate global
resources or will use library destructors (__attribute__(cleanup) with gcc)?
Eric
------------------------------------------------------------------------------
Go from Idea to Many App Stores Faster with Intel(R) XDK
Give your users amazing mobile app experiences with Intel(R) XDK.
Use one codebase in this all-in-one HTML5 development environment.
Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
http://pubads.g.doubleclick.net/gampad/clk?id=254741551&iu=/4140
_______________________________________________
ntfs-3g-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel
